[WIP] Multizone adjoints for turbomachinery

[joshkellyjak]

Proposed Changes

This is a cleaned up PR of the fixes needed for multizone adjoints for turbomachinery from the previous PR of @oleburghardt and I's work.

This hopefully is useable, so if you would like to test and report please feel free to contact me on Slack.

TODO:

• Fix eddy viscosity tape issue in CNSSolver
• Add turbomachinery objective functions + constraints
• Include MZ testcase

Related Work

Now closed PR #2317

PR Checklist

Put an X by all that apply. You can fill this out after submitting the PR. If you have any questions, don't hesitate to ask! We want to help. These are a guide for you to know what the reviewers will be looking for in your contribution.

• [ X ] I am submitting my contribution to the develop branch.
• My contribution generates no new compiler warnings (try with --warnlevel=3 when using meson).
• My contribution is commented and consistent with SU2 style (https://su2code.github.io/docs_v7/Style-Guide/).
• I used the pre-commit hook to prevent dirty commits and used pre-commit run --all to format old commits.
• I have added a test case that demonstrates my contribution, if necessary.
• I have updated appropriate documentation (Tutorials, Docs Page, config_template.cpp), if necessary.

[pcarruscag]

What is the difference between ending the preaccumulation before or after?

[pcarruscag]

Suggested change

	SU2_MPI::Error(string("OUTPUT_WRT_FREQ cannot be specified for this solver "
	SU2_MPI::Error("OUTPUT_WRT_FREQ cannot be specified for this solver "

[pcarruscag]

Hmmm I'm pretty sure it is possible

[joshkellyjak]

This is one for @oleburghardt

[oleburghardt]

Yes and no, writing the files is fine, but the continuation of the multi-zone discrete adjoint solver is erratic. My guess is that some indices aren't cleared properly before re-recording the tape. (Writing adjoint solution variables to file only, without re-recording at all and without sensitivities, might give us some hints.)
The debug mode could eventually pin down where exactly things go wrong.

[pcarruscag]

Why are these wrong but not the others that follow the same pattern?

[joshkellyjak]

This threw a tagging error, and if I remember correctly it's because in the other sections the variables it accesses are directly used in the solver, whereas in the Giles implementation they are not, creating a mismatch in tags.

[pcarruscag]

Can we fix it in a way that keeps a clear pattern for doing this type of thing?

[joshkellyjak]

I can give it a go

[oleburghardt]

I think this actually removes a potential tag tool error message (= dependency error message in this case), but it breaks the functionality, since e.g. BPressure = config->GetPressureOut_BC(); deletes the index of BPressure, as the config value appears as a constant.

I'd suggest to fix this, for now, by tracking the config class value instead of the local copy (?)

[oleburghardt]

@pcarruscag @joshkellyjak I'd suggest to solve problems in RegisterVariables in a different branch.

(The problem with the fix that I suggest is that, I think, some config class values are copied into the solver during initialization steps of the solver which might not be part of the recording. So it's also not a good solution, I'm afraid.)

[oleburghardt]

I think this actually removes a potential tag tool error message (= dependency error message in this case), but it breaks the functionality, since e.g. BPressure = config->GetPressureOut_BC(); deletes the index of BPressure, as the config value appears as a constant.

I'd suggest to fix this, for now, by tracking the config class value instead of the local copy (?)

[joshkellyjak]

@pcarruscag @joshkellyjak I'd suggest to solve problems in RegisterVariables in a different branch.

(The problem with the fix that I suggest is that, I think, some config class values are copied into the solver during initialization steps of the solver which might not be part of the recording. So it's also not a good solution, I'm afraid.)

Sure thing, this sounds reasonable from my perspective as it extends beyond the turbo world. For now it is maybe best to disable the tape tests in the regression workflow in that case?

[joshkellyjak]

I reran the APU turbocharger test case as the residuals looked a little strange on the regression workflow, given I have validated this compared to experimental data I would argue that this updating the values here and adding new solution data to the testcase repo is justified.

To fix this class of problem, ensure that at least one operand of the multiplication is explicitly cast to the larger integer type before the multiplication. This forces the multiplication to be evaluated in the wider type, preventing overflow that would occur if it were done in a narrower type first and only then converted.

In this specific case, the problematic expression is in the indexing term on line 110:

sendDonorVar[iSpan * nMixingVars + iVar] = buffDonorVar[static_cast<size_t>(iSize) * nSpanDonorVars + iSpan * nMixingVars + iVar];

Here, iSpan is an int and nMixingVars is (effectively) an unsigned short; their product is computed in int and only then converted to size_t as part of the whole index expression. To fix this without changing functionality, we should cast iSpan (or nMixingVars) to size_t before the multiplication so the product is computed in size_t. For consistency with the rest of the file, which uses static_cast<size_t>(...) for similar counts, we should use static_cast<size_t>(iSpan) * nMixingVars in both index expressions in that line.

No new imports or helper methods are needed; we only adjust the indexing expression in CMixingPlaneInterface::BroadcastData_MixingPlane inside CMixingPlaneInterface.cpp.

In general, to avoid this kind of overflow, ensure that at least one operand of the multiplication is explicitly converted to the larger integer type before the multiplication, so that the operation is performed in that wider type.

Here, the problematic term is iSpan * nMixingVars in the index expression:

sendDonorVar[iSpan * nMixingVars + iVar] =
    buffDonorVar[static_cast<size_t>(iSize) * nSpanDonorVars + iSpan * nMixingVars + iVar];

The best minimal fix is to cast one of the operands in the product to size_t so that the multiplication is performed in size_t, matching the other indices. We should apply this both in the left‑hand index and the right‑hand index, to keep the expressions consistent and avoid any risk of overflow. No new headers or functions are needed; we only add static_cast<size_t> around iSpan (or nMixingVars) in those products.

Concretely, in SU2_CFD/src/interfaces/cfd/CMixingPlaneInterface.cpp, in the inner loop starting at line 109, update the index expressions on line 110 to:

• sendDonorVar[static_cast<size_t>(iSpan) * nMixingVars + iVar]
• buffDonorVar[static_cast<size_t>(iSize) * nSpanDonorVars + static_cast<size_t>(iSpan) * nMixingVars + iVar]

This keeps behaviour the same for valid indices while ensuring the multiplication is computed in a wide type.