Skip to content

Fix preaccumulation error, improve recording of RANS solvers #2472

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Apr 15, 2025

Conversation

oleburghardt
Copy link
Contributor

@oleburghardt oleburghardt commented Mar 21, 2025

Proposed Changes

This contributes two algorithmic-differentiation-based fixes;

  • Add a missing preaccumulation input to the recording of SetRoe_Dissipation_FD. This may change derivatives as the current recording of this function is likely to be wrong if PREACC = YES.
  • Include muT (eddy viscosity) as input/output to the recording of RANS solvers. Since both muT and conservative variables go "on the same level" into the flow solver's Preprocessing (i.e. we cannot think of muT as a derived value from the conservative ones), this should be reflected in the recording. (Also, the debug mode developed in Tape recording debug mode #2442 will complain otherwise ;-)).
    This change does not affect derivative values – especially it does nothing about the circumstance that Preprocessing is based on an initial value of muT, whereas the flow solvers depend on an updated one – but it may clarify things and remove a source of future problems.

Related Work

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.

  • 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
Copy link
Member

@oleburghardt if everything is addressed can you resolve the conversations and merge please?

@oleburghardt
Copy link
Contributor Author

@oleburghardt if everything is addressed can you resolve the conversations and merge please?

Sure. CodeFactor is complaining about CSolver.hpp (complex code) which I've touched in a commit but which eventually remains unchanged. Forgot to ask during last dev meeting how we should handle it?

@pcarruscag
Copy link
Member

You can ignore that warning, It doesn't say what to do so we can't do anything.

@oleburghardt oleburghardt merged commit c9f098d into develop Apr 15, 2025
34 of 35 checks passed
@oleburghardt oleburghardt deleted the fix_small_AD_issues branch April 15, 2025 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants