Skip to content

fix TS adjoint#396

Open
SichengHe wants to merge 11 commits intomdolab:mainfrom
SichengHe:feature/ts-adjoint-fix
Open

fix TS adjoint#396
SichengHe wants to merge 11 commits intomdolab:mainfrom
SichengHe:feature/ts-adjoint-fix

Conversation

@SichengHe
Copy link
Collaborator

Purpose

Fix the time spectral (TS) adjoint, which was completely broken because the case (timeSpectral) block in initres_block (src/solver/residuals.F90) was wrapped inside #ifndef USE_TAPENADE, so Tapenade never generated adjoint code for the spectral time derivative terms. The generated AD code only handled case (steady), completely missing the TS contributions.

Changes:

  • Exposed case (timeSpectral) in residuals.F90 to Tapenade by moving the #endif boundary, with #ifdef USE_TAPENADE blocks to replace pointer access with direct flowDoms array access (required since Tapenade cannot differentiate through Fortran pointers)
  • Added reverse mode adjoint for TS in residuals_b.f90
  • Added forward mode tangent for TS in residuals_d.f90
  • Added state-only reverse mode for TS in residuals_fast_b.f90
  • Added test script tests/test_ts_adjoint.py for verification

Expected time until merged

Non-urgent. A week or two is fine.

Type of change

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (non-backwards-compatible fix or feature)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Documentation update
  • Maintenance update
  • Other (please describe)

Testing

Run with 2 MPI ranks on the NACA 64A010 Euler time spectral case (3 time intervals, pitching oscillation):
mpirun -np 2 python tests/test_ts_adjoint.py

Three tests are performed:

  1. Dot product test (forward vs backward consistency): relative error = 1.06e-15 (machine precision)
  2. Forward AD vs finite difference: residual derivative relative error = 4.03e-05; function derivatives (cl, cd, cmz) match to 1e-7 — 1e-8
  3. Adjoint sensitivity (evalFunctionsSens): all three adjoint solves (cd, cl, cmz) converge

Checklist

  • I have run ruff check and ruff format to make sure the Python code adheres to PEP-8 and is consistently formatted
  • I have formatted the Fortran code with fprettify or C/C++ code with clang-format as applicable
  • I have run unit and regression tests which pass locally with my changes
  • I have added new tests that prove my fix is effective or that my feature works
  • I have added necessary documentation

@SichengHe SichengHe requested a review from a team as a code owner February 27, 2026 00:53
Copy link
Member

@A-CGray A-CGray left a comment

Choose a reason for hiding this comment

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

Thanks for these fixes @SichengHe , can you please update your tests to use ADflow's existing RegTest class. See this file for an example.

Also, which version of Tapenade did you use to generate the new derivative code? I think the failing Tapenade check is probably due to a version mismatch, the test uses 3.16

Copy link
Member

Choose a reason for hiding this comment

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

Please put this file in the reg_tests directory

Comment on lines +16 to +17
# Add the test directory to path for reg_default_options
sys.path.insert(0, os.path.join(os.path.dirname(os.path.abspath(__file__)), "reg_tests"))
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't need this once file is moved

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

should be all good now.
Thanks for all the feedback! They are very helpful.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@A-CGray A-CGray self-requested a review March 19, 2026 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants