Fix Tapenade 3.16 reverse mode crash in SA routines#404
Open
yixin-cfd wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
This PR fixes reverse-mode AD regeneration with recent Tapenade 3.16 develop builds.
It addresses the issue discussed in #403, where
make -f Makefile_tapenade ad_reversecrashes while processing the SA turbulence routines insa.F90.Recent Tapenade 3.16 develop builds, including:
3.16-v2-13043.16-v2-1337crash during reverse TBR dependency analysis with errors like:
The crash is triggered by the SA routines:
Forward mode works, and reverse mode works for the other routines in the same file, such as
saResScaleandturbAdvection.The issue appears to be related to how recent Tapenade develop builds handle module-level global variables used by these routines. In particular, the SA model constants:
were module-level variables that were assigned inside
saSource,saViscous, andsaSolve. These values are only needed locally inside those routines, so this PR makes their definition and assignment local to the routines that use them.This avoids the Tapenade reverse-mode crash while preserving the original numerical behavior.
Expected time until merged
This is a small bugfix intended to restore compatibility with recent Tapenade 3.16 develop builds. I expect it should be reviewable within about a week.
Type of change
Testing
I tested reverse-mode AD regeneration using a recent Tapenade 3.16 develop build.
Before this change, Tapenade crashed when processing the SA routines:
with:
After this change, the SA routines no longer trigger the Tapenade reverse TBR dependency-analysis crash.
I also tested the reduced trigger cases identified in #403:
Both previously reproduced the crash when run alone with the full input file list present. With this change, Tapenade proceeds past these routines.
The change is intended to be numerically neutral: the constants
cv13,kar2Inv,cw36, andcb3Invare assigned the same values as before, but are now local to the routines that use them instead of being module-level global state.Checklist
ruff checkandruff formatto make sure the Python code adheres to PEP-8 and is consistently formattedfprettifyor C/C++ code withclang-formatas applicable