Bugfix for rotating frame sensitivities + add Tapenade checks #121
Conversation
….py is still hardcoded. This needs to be improved.
…gether. However, it seems from the older commits that these files we add in this commit are crucial. There are a few additions that should be located, but then it should (hopefully) work. Most likely, the error that remains to be fixed now is that some functions already in use are not differentiated correctly (just as in the previous case with inviscidcentralflux()). Hopefully a fix is coming up shortly.
…n hardcoded. Note; this commit will not work without the proper DVGeometry.py file from PyGeo...
… need slipvel() and then we need to move them all to adjointextra_d.f90
…ve been added. Weird
…e possible files containing the error for master_b has been narrowed down.
|
Alright @anilyil @SichengHe @eirikurj , I tried to cleanup the tapenade makefile by removing some of the dependent variables that "did not look dependent" (i.e. I eyeballed), I tested the adjoint locally but I missed all the jacobian vector product failures happening right now. As @nwu63 mentioned, there's no point in removing stuff by trial and error because even if all the tests pass, we might be missing some zeroed-out partials. My hot take is that ADflow maintainability in this sense has been already compromised, and we should make a separate effort to fix that and 1) cleanup |
I stand by this, but I managed to fix and complete the cleanup of these routines. All other additions look targeted for the bugfix! |
Good call, I had that in mind but then forgot to actually fix it. Check it out now and let me know if the comments make sense |
|
I want to do another quick pass some time later this week but I think the PR is good to go. Why were we not getting failures due to the extra |
I think it was because of the small contributions, there are no unit tests but we are definitely covering that part of the code with the rotating test we added. Local dot-product tests match to machine precision! |
There was a problem hiding this comment.
This is good to merge for me. Do we want to wait for @SichengHe's review?
|
I am fine with merging it. I just hit re-request on everyone who was involved. I think @marcomangano can do the honors |
|
Go for it @marcomangano and please make the release afterwards as well |
Purpose
Long overdue, this PR addresses the sensitivities inconsistencies on the current ADflow release when setRotationRate() is included to add a rotation component to the wall BC. The source code fixes (up to commit 0d39d74) have been initially implemented by @Aagaard87 to perform high-fidelity aerodynamic shape optimization of a wind turbine using MACH-Aero.
The old dev-branch with the fixes has been rebased in the current ADflow release, and the code has been further cleaned for clarity. See the discussion on #81 for more details. The final code in this PR is mostly the same as in #81 , but the commit history has been fixed to preserve the development history.
An additional regression test has been added for the real build, while the complex build cannot be performed yet as convergence is obtained with the ANK solver, which is not complex-safe yet.
@SichengHe put a big debugging effort into restructuring the code to have the relevant subroutines relocated in the appropriate file, verify the consistency of the AD process, and ultimately debugging derivatives inconsistencies between adjoint and complex-step derivatives.
Note that this PR also introduces the Tapenade checks on ADflow - currently using version 3.10
Type of change
Testing
A new test (real only) has been added to
test_adjoint.py. The.reffile might need to be retrained on a docker image for consistency. Also note that the test will currently fail because the dedicated mesh has not been uploaded yet.Checklist
flake8andblackto make sure the code adheres to PEP-8 and is consistently formatted