Axisymmetric near-axis fixes using L’Hôpital-consistent formulations - #2715
Conversation
|
Hi @raghava-davuluri, thanks for this fix! Does this case need additional regression tests or is it covered by the existing regression tests for axisymmetric flow? |
|
This is addressing #2438 It helps in most situations, but dips in axisymmetric heating cases can still be present for some meshes. So @raghava-davuluri is doing a pull request to help get more eyes on it. |
|
@raghava-davuluri @bigfooted, reading up on this more, L'Hopital's rule may be more suitable for cell-centered and less so for vertex-centered codes. And that all the residuals should still be zero at the symmetry line. Here is an example of the axisymmetric sources along the surface. I would expect the energy and momentum-x to smoothly go to zero at theta=0 instead of a discontinuity
|
|
I'd say the source terms should not jump close to r=0, but they are allowed to be nonzero. |
|
@raghava-davuluri can you update the branch? Even though there is still work to be done I think we should put this in the code since it is an improvement over what we currently have. |
1422a8a to
914bebc
Compare
|
@bigfooted Thanks! I have updated the branch with the latest develop. The regression-test workflow is currently awaiting maintainer approval to run. |
|
Hi @raghava-davuluri is there an existing regression test that can be used? How large is the testcase that you used for the image? |
|
@raghava-davuluri can you check the failed regression tests and if it makes sense, update the residuals? |
|
@bigfooted I am working on that. I will update you soon. |
67520e7 to
f8006e0
Compare
|
@bigfooted I’ve updated the branch, resolved the conflicts, and updated the affected regression values. The workflows are now awaiting maintainer approval. Could you please approve them when you get a chance? Thanks! |
|
@bigfooted, I’ve pushed another update to address the regression test issue. The workflow is awaiting approval again. Hopefully this is the last time and everything works this time! Thanks again for your help. |
|
Hi @raghava-davuluri OK, you're the next PR to get merged, great to have this fix finally in! |
|
@raghava-davuluri can you check that the ion regression testcase converges if you let it run for more iterations? one of the density residual values is increasing but maybe it's temporary? Preferably all regression tests should converge if you run it to completion. |
|
@bigfooted I looked into this further and found the source of the ion_gy regression. The change I made in ComputeAxisymmetricAuxGradients() is also affecting NEMO, since both CEulerSolver and CNEMOEulerSolver call this common routine. I confirmed this locally by restoring only this function to the upstream version, after which the ion_gy regression test passes again. I’ll work on separating the new axis treatment from the existing NEMO behavior. Please give me a day to resolve this, and I’ll leave another comment here once the fix is pushed and ready for CI approval. |
|
If the method is correct for NEMO then that's fine, it should just be checked that the case still converges with this different type of source term. If it doesn't then I hope it's sufficient to use a lower CFL. |
|
@bigfooted Thanks. The axisymmetric treatment in this PR is currently only intended for the standard SU2 solver; the corresponding NEMO treatment has not been included yet. Since SU2 and NEMO share the same auxiliary-gradient function, the change was also affecting NEMO and caused the ion_gy regression failure. I’ve now added a condition to exclude NEMO from this treatment for the time being, and verified the NEMO axisymmetric regression cases locally. Hopefully this is the last time I’ll need to ask you to approve the CI tests! |
|
OK, thanks for the clarification. there is now a conflict, can you check? I guess it needs to be merged with latest develop again. |
d415566 to
503e6fd
Compare
|
@bigfooted Done. I rebased the branch onto the latest develop and resolved the conflict. I also rebuilt and rechecked the relevant axisymmetric cases, including the NEMO cases, and they pass locally. The workflows are awaiting approval again. Thanks! |
|
only axi_rans_air_nozzle_species regression test still fails... |
|
@bigfooted Thanks. I have applied both suggestions locally, rebuilt, and the axisymmetric nozzle test still passes. I also pulled the missing solution_flow_species.dat from the develop branch of the TestCases repository and reproduced the axi_rans_air_nozzle_species failure locally. I get exactly the same values as CI: [-1.676190, 3.896581, -2.912396, 5.762790, -3.565909, 0.000000]. Since you mentioned this change is due to the multigrid solver changes, should I update the regression values to these values before I push again? |
|
Hi, yes please update the regression test values and then we are good to go. |
|
@bigfooted Thanks. I updated the species regression values with the locally reproduced values and also applied both of your suggestions. I rebuilt and reran the relevant tests locally, and they pass. I’ve pushed the changes now. Hopefully we’re good to go! |
|
@bigfooted The failure was from the AD build because the ternary had a CoDi expression (1.0/r) on one side and a double (0.0) on the other. I replaced it with yinv = 0.0 followed by yinv = 1.0/r inside the existing if (r > EPS), so the logic remains the same. I pushed the fix now. Could you please approve the CI tests again? Hopefully this is the last round! 😄 |
|
congrats ! :-) |
|
Thanks for all the help, Nijso! 😄 I think writing the code was easier than getting the pull request through! |

Proposed Changes
Give a brief overview of your contribution here in a few sentences.
Related Work
Resolve any issues (bug fix or feature request), note any related PRs, or mention interactions with the work of others, if any.
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.
pre-commit run --allto format old commits.