-
Notifications
You must be signed in to change notification settings - Fork 1k
Time-accurate discrete adjoint gradients for unsteady problems with grid movement #766
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
Changes from all commits
Commits
Show all changes
34 commits
Select commit
Hold shift + click to select a range
9431716
tidy comments and indentation
cvencro 7caf5ea
correct column look-up in restart file
cvencro 7042b14
consistent definition for adjoint boolean
cvencro 597c133
revert incompressible ale changes to prepare for PR
cvencro b805962
Revert excecutable file permission for cpp/hpp/inl files.
TobiKattmann 8a4c291
Req. changes for PR766. GridVel allocation, indentation, error msg.
TobiKattmann 85047c4
Renamed grid_movement_bool & fixed error catch for dynamic mesh+discAdj.
TobiKattmann c9f002b
Wrapped GridVel allocation in discAdj conditional. PR766.
TobiKattmann 8e04358
Test without additional secondary and main recordings at the start of…
cvencro 272ee6a
Merge branch 'feature_dynamic_mesh_discrete_adjoint' of https://githu…
cvencro 585f702
Comment out get, set of additional containers and initial test using …
cvencro ae7460c
Merge remote-tracking branch 'origin/develop' into feature_dynamic_me…
TobiKattmann 1d1275a
Comment out unused containers
cvencro b23a2f7
FD grid velocity calc and remove old grid velocity containers
cvencro 28b6802
Merge branch 'feature_dynamic_mesh_discrete_adjoint' of https://githu…
TobiKattmann b1020b4
Merge remote-tracking branch 'origin/develop' into feature_dynamic_me…
TobiKattmann a3a8691
Removed unnecessary GridVel containers and methods for ALE discrete a…
TobiKattmann a2ca1b5
Removed unintentionally added blank lines.
TobiKattmann 500b173
fix needed to allow update of old geometry for dynamic fsi
cvencro fd11480
Test case added - discrete adjoint of transonic pitching airfoil
cvencro 101220e
Merge remote-tracking branch 'origin/develop' into feature_dynamic_me…
TobiKattmann 5694277
Merge remote-tracking branch 'origin/feature_dynamic_mesh_discrete_ad…
TobiKattmann aa52484
Merge remote-tracking branch 'origin/develop' into feature_dynamic_me…
TobiKattmann bdead2e
Merge remote-tracking branch 'origin/develop' into feature_dynamic_me…
TobiKattmann 00ed905
Removed unused variable.
TobiKattmann f4974a2
Change of conditional in front of Restart_OldGeometry.
TobiKattmann aaecc65
Merge remote-tracking branch 'origin/develop' into feature_dynamic_me…
TobiKattmann 006e015
Merge remote-tracking branch 'origin/develop' into feature_dynamic_me…
TobiKattmann c3a6e7d
RestartOld_Geometry conditional fix for FSI cases.
TobiKattmann 2d8b0ea
Re-add code deleted during merge conflicts.
TobiKattmann 0d21ee0
Added serial+parallel reg test for pitching discAdj.
TobiKattmann 96dec9c
Removed doubled SecondaryRecording, which errors unsteady adjoints.
TobiKattmann 2d56130
Revert travis. Last little change for PR766 Regression test.
TobiKattmann d1262f1
Changed DV_* cfg lines such that parallel_computation.py works.
TobiKattmann File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
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
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This
SecondaryRecordingis now removed entirely, as it introduces a bug for the unsteady adjoint. The Volume Mesh Sensitivity setting is a+=in each physical timestep for unsteady cases andSecondaryRecordingis also called inPostprocess()right after theRun(). I.e. if it is called multiple times in one physical timestep the sensitivity adds up. With this code modificationSecondaryRecordinggets only called once per timestep in Postprocess.For steady cases the implications are: The memory for the secondary recording gets allocated after the 'flow adjoint iteration' ... so it could be really annoying if the simulation crashes due to lack of memory after waiting for convergence. More importantly: You cannot use an intermediate restart file for e.g. DOT_AD as it is does not hold any/correct sensitivities. So it might be better to only disable this conditional for unsteady computations as I see the advantage of having this for steady computations. What do you think @rsanfer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I honestly prefer it that way as it is much faster for steady-state computations in moving meshes (a lot less unnecessary recordings). In fact that was my first implementation, but then I added this SecondaryRecording after discussion with @talbring, because of the drawbacks you mention. No 100% what's the best approach...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My only concern is the memory thing, because usually the recording with coordinates as inputs takes more memory than the one with the solution variables. I am not so worried about the intermediate restart files, since you can just do a restart to get the correct sensitivity values.
Lets do it without the SecondaryRecording. If it really leads to problems, we can always change it back.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, sounds good to me.