Skip to content
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

Streamwise Periodic restarts using flow.meta + Multizone PerSurface output #1527

Merged
merged 28 commits into from
Feb 2, 2022

Conversation

TobiKattmann
Copy link
Contributor

@TobiKattmann TobiKattmann commented Jan 25, 2022

Hi,

  • main thing of this PR is supposed to be the working restarts for prescribed massflow in streamwise periodic simulations (primal and adjoint). The massflow is achieved by dialing a pressure drop which is written and read to/from the flow.meta file. 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)
  • I added a weight to heat-solid zone OF's
  • when pressure_drop is used, the first two entries of MARKER_ANALYZE are used as out-inlet but more than two markers are allowed. This comes in handy with FLOW_COEFF_SURF imo
  • Adding per surface output for multizone cases. Was not working before in the way that _SURF[0] fields were reported to be ignored. A regression test is updated for that.
  • Little fix to the regression system where Testcase.py had unsuitable default values such that failing reg tests were reported passing

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

  • I am submitting my contribution to the develop branch.
  • My contribution generates no new compiler warnings (try with the '-Wall -Wextra -Wno-unused-parameter -Wno-empty-body' compiler flags, or simply --warnlevel=2 when using meson).
  • My contribution is commented and consistent with SU2 style.
  • I have added a test case that demonstrates my contribution, if necessary.
  • I have updated appropriate documentation (Tutorials, Docs Page, config_template.cpp) , if necessary.

…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.
@pr-triage pr-triage bot added the PR: draft label Jan 25, 2022
TobiKattmann and others added 2 commits January 25, 2022 18:43
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
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.
In multizone everything is done again + attaching the zone indices. So one
had to do the AddHistPerSurf and SetHistPerSurf for multizone again.
@TobiKattmann TobiKattmann changed the title Streamwise Periodic restarts using flow.meta Streamwise Periodic restarts using flow.meta + Multizone PerSurface output 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.
TestCases/TestCase.py Outdated Show resolved Hide resolved
….. which I believe is not clever programming, especially for regression tests.
@TobiKattmann TobiKattmann marked this pull request as ready for review February 1, 2022 15:30
@TobiKattmann
Copy link
Contributor Author

I am not really sure whether the chore-tag was correct here, it adds features that should have been there in the first place but feature-tag prob is the better description

Copy link
Member

@pcarruscag pcarruscag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good addition 👍 minor comments below.

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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warn instead of error then? (when nZone != 1)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, done in 75746ac

SU2_CFD/src/output/CMultizoneOutput.cpp Outdated Show resolved Hide resolved
SU2_CFD/src/solvers/CSolver.cpp Outdated Show resolved Hide resolved
Comment on lines 187 to 189
vector<string> Marker;
if (group == "FLOW_COEFF_SURF")
Marker = Marker_Analyze;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

find a way to avoid copying a vector of strings

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... found a working *|& permutation that does the job in 2cbbe23

SU2_CFD/src/output/CMultizoneOutput.cpp Outdated Show resolved Hide resolved
TobiKattmann and others added 3 commits February 2, 2022 13:11
Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>
Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>
Common/src/CConfig.cpp Outdated Show resolved Hide resolved
Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>
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
@TobiKattmann
Copy link
Contributor Author

Thanks all the review comments (for the second time today 😉 ) @pcarruscag 💐 Merging this in now 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants