Zipper fix and some other modifications #12
Merged
Conversation
This file contains 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
The overset adjoint sensitivity accuracy (dCD/dTwist) improves from 2-3 digits to 6-7 digits, based on the DPW4 10 million-cell-case.
…rder fix. We don't know the reason for the order dependence, but it improves gradient accuracy to expected levels on most cases. See the zipper mesh bug issue for more info.
Still needs to be validated
Added modifications to om_states_comp so that we dont waste restarts. Also re-setting ordersConverged variable before each steady state solver call because the value might be low if the previous iteration did not converge past relaxEnd.
Minor additions to .gitignore.
Since we actually modify the update vector in some cases, vecGetArrayReadF90 should be replaced with vecGetArrayF90, and the corresponding restore call should be modified similarly. However, because we have a collective MPI call right after these, the previous implementation was OK and this did not cause any issues. Modifying these calls anyways to prevent possible errors in the future.
gkenway
reviewed
May 24, 2019
| @@ -1014,6 +1014,9 @@ subroutine solveState | |||
| ! call, "Iter" and "Iter Total" will both display 0. | |||
| approxTotalIts = 0 | |||
|
|
|||
| ! we need to re-set the orders converged to 16 as it might have been modified in the previous iteration | |||
| ordersConverged = 16.0_realType | |||
There was a problem hiding this comment.
Why is this necessary exactly? The ordersConverged gets set unconditionally below anyway.
|
Also added a fix for an edge case, where ANK segfaults with Euler and turbKSP turned on. If we are not doing RANS, ank setup will not try to create the objects for the turbKSP |
|
Is something here still pending? @anilyil do we need additional reviews, new regression tests for these cost functions? |
|
@anilyil did you see/address gaetan's comment? |
|
Yeah, you can see my reply to him after his comment. I tried to explain the logic behind having that there. It only affects a very small subset of cases where we have an actuator region, and restarting from a partially converged solution. |
ewu63
pushed a commit
that referenced
this pull request
May 19, 2020
Zipper fix and some other modifications
Open
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.
Fixed the call order on the reverse zipper integration.
Added area averaged total and static pressure integration.
Bug fix with area integration with overset meshes.
Bunch of additions to .gitignore.
Minor fix in the ANK solver physicality check.
Modified how the restarts are handled with the OpenMDAO states component.
Minor fix for edge case when we are using actuator regions and the previous iteration was partially converged.