Proposed Changes

The velocity component used in the axisymmetric residual is now considered as an input for preaccumulation in the residual computation. When not considered, the use of axisymmetry and the SST model will result in wrong sensitivities when using the discrete adjoint method.

I also wanted to note that there does not seem to be any treatment of axisymmetric cases for the SA model. Is this correct or does it make sense to include a warning? We could, for example, do that in the context of this pull request.

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 the '-Wall -Wextra -Wno-unused-parameter -Wno-empty-body' compiler flags, or simply --warnlevel=2 when using meson).
• [ x] My contribution is commented and consistent with SU2 style.
• 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.

Oh and last thing, add yourself to AUTHORS.md

Thanks for the fast reply! I changed the hardcoded 2 (just as a remark: the hardcoded values also appear in the calculations), added myself as an author and inserted some lines for throwing an error if the issue in #1565 occurs. I did not know exactly where to put it best.

Concerning a regression test: I strongly support the idea of introducing an axisymmetric regression test. However, I was using a testcase from @bigfooted , and I never set up such a test case on my own. There do not seem to be any axisymmetric pipe setups in the Testcases folder so far. @bigfooted , could we maybe use your mesh for the jet flow test case and, if necessary, switch to a standard flow setup?

Yep that's what I meant, replace it in the calculations as well please, and also in PrimVarGrad, the leading 2 is "velocity + 1" and the leading one is "velocity".
The reason for this replacing is to make the turbulence sources compatible with NEMO.

We have some small flat plates here and there, I can convert one to axisymmetric.

Okay, done. Sorry, I read over that comment before. Hope I picked the correct component in PrimVarGrad.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fast reply! I changed the hardcoded 2 (just as a remark: the hardcoded values also appear in the calculations), added myself as an author and inserted some lines for throwing an error if the issue in #1565 occurs. I did not know exactly where to put it best.

Concerning a regression test: I strongly support the idea of introducing an axisymmetric regression test. However, I was using a testcase from @bigfooted , and I never set up such a test case on my own. There do not seem to be any axisymmetric pipe setups in the Testcases folder so far. @bigfooted , could we maybe use your mesh for the jet flow test case and, if necessary, switch to a standard flow setup?

Yes, you can use that mesh. It can be used for pipe flow setup and jets with coflows, so we can reuse it in different testcases as well if needed. But any simple rectangular mesh is fine, so a mesh from the existing testcases as @pcarruscag mentions would also work.

I was aware of that test case. But I did not use it for the verification of sensitivities since I could not manage to set up a working derivative computation (different combination of markers/objective functions were leading to a memory error). However, I could use it to set up a regression test for the comparison of residuals.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the confusion. I must have mixed something up yesterday - the test case was crashing directly for SU2_CFD_AD when I set MARKER_MONITORING. But now it is working. So I think we can stick to the regression case and the way I set it up. Thanks again for the reviews!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this fix Lisa 💐 The code bits look good to me.

When it comes to the cfg I much prefer (and that is also a bit current SU2 strategy) stripping all the comments away instead of a few ones you want to highlight. This cfg is of course very close to an existing one and I am wondering whether we can make both cases work with just one cfg.

The first step would be to not remove MATH PROBLEM as then the usage of SU2_CFD or SU2_CFD_AD indicates primal or adjoint.

The second step is to get rid of the restart which I guess we could do with this part from Testcase.py

# Indicate whether to disable restart
 self.no_restart = False

Did you change any other options? If not I guess this is the way how we could avoid duplicating the cfg. So it might be more appropriate to not apply the suggestions here on github but to do it on your machine and test it, and if it works push 👍

This is confusing me a bit. Before I set it up: Do you want me to use the original config (without the _adj), remove the comments and the model and then run SU2_CFD_AD in the regression test?

In the first instance I would just try to run the new and original Testcase with just the original config. And from what I can see you need to do the 2 steps mentioned above (Remove MATH_PROBLEM and use SU2_CFD_AD directly for the adjoint test + use the no_restart = True option for the reg test). If nothing else (apart from Math_Problem and restart_sol) was changed then this should work without an extra *_adj.cfg.

In a next step (which is then more of a cosmetic, optional nature) you could strip the comments and unnecessary options from the cfg

27d35a2 -> awesome.

Concerning the option(-comment) stripping; I am still in favor, although @FlorianDm is the original author. Especially I would strip the AOA, SIDESLIP_ANGLE, CFD_ADAPT_PARAM, SYSTEM_MEASUREMENTS, MESH_FORMAT as they seem superfluous to me.

But for the sake of keeping this PR concise the current state is fine as well 👍