Streamwise Periodic restarts using flow.meta + Multizone PerSurface output#1527
Merged
TobiKattmann merged 28 commits intodevelopfrom Feb 2, 2022
Merged
Streamwise Periodic restarts using flow.meta + Multizone PerSurface output#1527TobiKattmann merged 28 commits intodevelopfrom
TobiKattmann merged 28 commits intodevelopfrom
Conversation
…h that setting. Note though that the comment in the pressure update computation of the streamwise periodicity still holds: Currently the pressure update for massflow is omitted in the first iteration, as the function is called in the Preprocessing phase which leads to multiple calls during 1 iteration. Ideally this whole method is called in another place or a setup has to be found which prevents multiple calls.
Not working yet. E.g. FLOW_COEFF_SURF[0] is not longer reported as ignored hist output filed but all values for these in the history file are zero. Not sure why tbh
TobiKattmann
commented
Jan 28, 2022
CConfig checks whether the DV_MARKERs appear in the marker list. But for multizone cases it can happen, that the marker to be deformed are just in the zonal configs. The check would fail although everything is alright. To check it right one would need to check this after all zonal cfgs were read and then check it once with a loop over all cfgs to see whether DV_MARKER appears somewhere there.
…to massflow_flowmeta
In multizone everything is done again + attaching the zone indices. So one had to do the AddHistPerSurf and SetHistPerSurf for multizone again.
TobiKattmann
commented
Feb 1, 2022
TobiKattmann
commented
Feb 1, 2022
TobiKattmann
commented
Feb 1, 2022
When using WALL_TIME as the first SCREEN_OUTPUT field, in Testcase.py, the code would never reach the iter_missing=True block as the line would produce a ValueError for every line. And because the default was iter_missing=False and passed=True ... the test reports passed. Setting the defaults to the non-passed version fixes that issue.
TobiKattmann
commented
Feb 1, 2022
….. which I believe is not clever programming, especially for regression tests.
Contributor
Author
|
I am not really sure whether the |
pcarruscag
reviewed
Feb 2, 2022
Member
pcarruscag
left a comment
There was a problem hiding this comment.
Good addition 👍 minor comments below.
Common/src/CConfig.cpp
Outdated
Comment on lines
5739
to
5740
| if(!found && (nZone==1)) { | ||
| SU2_MPI::Error("DV_MARKER contains marker names that do not exist in the lists of BCs in the config file.", CURRENT_FUNCTION); |
Member
There was a problem hiding this comment.
Warn instead of error then? (when nZone != 1)
Comment on lines
187
to
189
| vector<string> Marker; | ||
| if (group == "FLOW_COEFF_SURF") | ||
| Marker = Marker_Analyze; |
Member
There was a problem hiding this comment.
find a way to avoid copying a vector of strings
Contributor
Author
There was a problem hiding this comment.
... found a working *|& permutation that does the job in 2cbbe23
Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>
Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>
TobiKattmann
commented
Feb 2, 2022
pcarruscag
reviewed
Feb 2, 2022
pcarruscag
approved these changes
Feb 2, 2022
Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>
TobiKattmann
commented
Feb 2, 2022
This is due to the previously merged PR #1530 where an NDIME error in the mesh was fixed (where the reg test values already changed) and as this PR makes the same case a restarted one... the values have to be adapted again
Contributor
Author
|
Thanks all the review comments (for the second time today 😉 ) @pcarruscag 💐 Merging this in now 👍 |
5 tasks
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
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.
Hi,
flow.metafile. That file was used mainly for fixed CL stuff as far as I am aware of. The story is not done though really with this one change. There is still the restart-should-contain-everything-in-the-first-iteration-for-the-adjoint-taping issue which is currently not the case ... and not super straightforward to solve -> see c113541 description for more info. (EDIT: I will not tackle the massflow adjoint part in this PR, as with the PerSurf output it contains enough stuff which I like to have merged)weightto heat-solid zone OF'spressure_dropis used, the first two entries ofMARKER_ANALYZEare used as out-inlet but more than two markers are allowed. This comes in handy withFLOW_COEFF_SURFimo_SURF[0]fields were reported to be ignored. A regression test is updated for that.So I will continue fiddling a little bit with massflow restarts but i already successfully optimized with that setup so it is not completely useless.
Related Work
#773
PR Checklist