Fix grid deformation related bugs + other minor issues.#1126
Fix grid deformation related bugs + other minor issues.#1126pcarruscag merged 9 commits intodevelopfrom
Conversation
|
Thanks for giving it a try but on both machines I get a segfault at roughly the same spot (following the screen output). Unfortunately the stack traces I get are somewhat useless (I literally get Tested with your this branch on coupled_cht/incomp_2d |
|
Hmmm can you run |
On my private machine with it is 2.32 and on the cluster 2.17. I get |
|
But that case is not discrete adjoint, are you running SU2_CFD or SU2_CFD_AD? |
|
I am running SU2_CFD_AD. SU2_CFD runs fine. For completeness: the case is also this tutorial case |
|
Ok, I replicated the problem in the meantime, can you try a debug_optimized build (that seems to work for me regardless of the algorithm). |
|
Yep that works! |
|
I guess I fixed it, somehow the two "DonorInfo" classes in different cpp files got mixed up. EDIT: Rightly so apparently: https://stackoverflow.com/questions/10671956/same-class-name-in-different-c-files |
WallyMaier
left a comment
There was a problem hiding this comment.
Thanks for continuing to improve the code @pcarruscag.
This looks good to me
TobiKattmann
left a comment
There was a problem hiding this comment.
Thanks for fixes. I have a few minor comments
|
|
||
| /*--- Find k closest points (need to define the comparator inline or debug build give wrong results). ---*/ | ||
| /*--- Find k closest points. ---*/ | ||
| partial_sort(donorInfo.begin(), donorInfo.begin()+nDonor, donorInfo.end(), | ||
| [](const DonorInfo& a, const DonorInfo& b) { |
There was a problem hiding this comment.
Ok so here it took the struct DonorInfo from CIsoparametric.cpp as it is "first come first serve" kind-of rule used by the linker. Now you moved both structs into the class-namespaces where they cant start no trouble. Is it important that they are private?
Oh, and if I had any big bucks to spare i would surely send some over 💸 😉
There was a problem hiding this comment.
No the main thing is the namespace, an anonymous namespace in the cpp files would work as well.
| } | ||
| } | ||
|
|
||
| /*--- If the gradient of the objective function is 0 so are the adjoint variables. ---*/ |
There was a problem hiding this comment.
dont run the MZ adjoint if ||del(OF)|| = 0 do you mean ||der(OF)|| in the commit message for derivative?
As this is a sanity check before dumping computational resources into a useless computation would it make sense here to write an output message?
And I dont really understand this check so I will ask later in the dev-meeting
There was a problem hiding this comment.
For completeness here, because the RMS residual is computed in "Iterate" it serves as a norm of the objective function gradient when that is what is extracted by "Iterate" for a future time-domain implementation this may need to change.
The motivation for this change was that running the adjoint with tiny values of OF gradient actually caused it to diverge.
…move periodic boundaries
|
I looked over the last added commit and I would be happy to have this merged soon to get those juicy debug runs of SU2_CFD_AD with multizone cases. |
Proposed Changes
Fix the output of FFD paraview files;
Maybe fix #1123 (@TobiKattmann give it a try if you can)
Deprecate VISUALIZE_VOLUME/SURFACE_DEF options for the reasons in #942;
Implements "HOLD_GRID_FIXED" for CMeshSolver.
PR Checklist