Deprecate options that did not work since v7.0.0, and other minor cleanups by TobiKattmann · Pull Request #1129 · su2code/SU2

TobiKattmann · 2020-12-10T22:46:20Z

Proposed Changes

Hi all,
Following a bit the idea of #942 this PR mostly gets rid of stuff.

WRT_RESIDUALS unused
WRT_LIMITERS unused
WRT_CON_FREQ_DUALTIME only used in legacy output ... so I thought I just go for it
removed SetMeshFile function as it not used anywhere and CSU2MeshFileWriter is the new cool kid on the block 🛴
Added some options to the config_template which were missing
Added output of the mpi rank to volume output of (in-)compressible and heat solver. I used that for debugging mostly but it might be interesting for others as well. Yes yes I know, one could think of putting COORDS, MESH_QUALITY (except for FEM) and now RANK into a centralized place to avoid duplication... next PR... I promise ... maybe
and a few tiny things
EDIT: yeah and some more options after my initial PR, see CConfig.cpp in files changed for all

There is a lot of other things to deprecate so feel free to add here.

I have to check the tutorials for these options as well... ✔️ done and open PR (see below) but not merged yet to wait for reviews. This fails to reg tests of tutorials.

(p.s. github now has a dark mode, which makes writing this on Thursday nights much nicer 🕶️)

Related Work

none

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. not necessary
I have updated appropriate documentation (Tutorials, Docs Page, config_template.cpp) , if necessary. not necessary

…put.

pcarruscag · 2020-12-11T10:51:56Z

Dark mode?
"Is it possible to learn this power?"

TobiKattmann · 2020-12-11T11:07:00Z

Dark mode?
"Is it possible to learn this power?"

🧙 Sure, if you just go to your front page (i.e. just github.com) there should be a big button directly on the right side.
Or Settings->Appearance->Dark. Enjoy :)

pcarruscag · 2020-12-11T12:00:09Z

What is the equivalent of DUAL_TIME in the TIME / OUTER / INNER versions we have now? TIME?
(so that we can auto-replace the stuff that is used in the testcases without breaking things too much)

With all this deprecation the next version number should probably be 7.1.0 even if I would call these things bug fixes from v6.2 to v7.0.
Going 7.0.8 -> 7.1.0 also saves the awkward 7.0.9 -> 7.1.0 (which would make people wonder if we know how version numbers work).

…_minor_cleanups

…into chore_minor_cleanups

…nups

TobiKattmann · 2020-12-11T13:12:32Z

Well thanks for fixing the typo.

It seems we use OUTPUT_WRT_FREQ for steady and unsteady simulations. Therefore the testcase cleanup was a bit more involved. For steady cases WRT_SOL_FREQ had to be replaced and for unsteady ones WRT_SOL_FREQ_DUALTIME to follow the author's intention... and I have the feeling that nearly every testcase had these options in it.

I have to admit I don't understand why 7.0.9 would be awkward 🤔

TobiKattmann · 2020-12-11T13:17:07Z

I also removed your options from the tutorials @pcarruscag : only WRT_CSV_SOL was used in two cases

pcarruscag · 2020-12-11T13:54:40Z

Because 7.0.9 + 1 = 7.0.10 xD
Thanks for taking care of the tutorials, I have a few more options coming.

Silly question, using OUTPUT_WRT_FREQ = 1 to output on every time iteration, will it cause outputs on every inner iteration for some unsteady problems that were using the DUALTIME options?
(I don't know much about unsteady options...)

TobiKattmann · 2020-12-11T17:42:17Z

Silly question, using OUTPUT_WRT_FREQ = 1 to output on every time iteration, will it cause outputs on every inner iteration for some unsteady problems that were using the DUALTIME options?
(I don't know much about unsteady options...)

For unsteady cases output is only written at the end of each "time"-iteration (if the OUTPUT_WRT_FREQ is matched), never inside the inner-iterations. OUTPUT_WRT_FREQ works differently for steady and unsteady. So that is not an issue.

pcarruscag · 2020-12-11T18:28:52Z

@TobiKattmann are you going after WRT_CON_FREQ now? (or at some point)

TobiKattmann · 2020-12-11T23:21:47Z

I was too lazy on that. Sorry, I should have done that that in the first place 😞

As there were only a few non-defaults I would have gone with:
find . type f \( -name '*.cfg' \) -exec sed --in-place '/WRT_CON_FREQ= 1/d' {} \+
and changing the rest manually (exercising the muscle memory a bit), but that handy script might get some additional use for other options ;)

EDIT: This suggestion above is bad how I just noticed -> sed here deletes each line that contains WRT_CON_FREQ= 1 which rightfully contains WRT_CON_FREQ= 10 etc! One has to wrap the search expression in \b to only deletes lines that exactly match. The command becomes:
find . type f \( -name '*.cfg' \) -exec sed --in-place '/\bWRT_CON_FREQ= 1\b/d' {} \+

If you have some more options to deprecate in mind I can take care of some. Maybe I try to hunt down a few more over the weekend 🏹

TobiKattmann · 2020-12-11T23:28:45Z

That seems to have confused git, maybe because of line endings on some testcases or something.

you prob mean that some files a "rewritten" completely. Hm seems to happen whenever a file ends on WRT_CONV_FREQ and does not have the default value.

This reverts commit d8dcfb0.

TestCases/hom_euler/NACA0012_5thOrder/fem_NACA0012.cfg

…nfig.

TobiKattmann · 2020-12-13T00:00:48Z

SU2_CFD/src/output/output_structure_legacy.cpp

  bool output_surface = (config->GetnMarker_Analyze() != 0);
  bool output_comboObj = (config->GetnObj() > 1);
-  bool output_per_surface = config->GetWrt_Surface();
+  bool output_per_surface = true;


Obviously this kind of change (there are just 3 in this file) takes away functionality from users of output_structure_legacy. In these cases I was always generous 👑 and enabled stuff which potentially clutters output but I am unsure whether anybody even uses this at this point

EDIT: now it is just WRT_SURFACE

SU2_CFD/src/output/output_structure_legacy.cpp

…ests.

TobiKattmann · 2020-12-13T22:55:42Z

Ok I am happy with the changes as they are right now. I've looked over the diff once again and I have no more comments (obviously more regarding @pcarruscag commits than my own). Btw thanks for being attentive regarding my mistakes and of course for all the added commits 👍

pcarruscag · 2020-12-14T18:55:11Z

Thank you for taking the lead (this is boring stuff, but less so if more people do it).
I also cannot think of any more options, but I might try to do something about the objective functions.
EDIT: on another PR perhaps, so if want to hit that button...

TobiKattmann · 2020-12-14T23:13:24Z

By the way, I noticed you didn't squash-merge. Any reasons or just forgotten? No big deal though

pcarruscag · 2020-12-14T23:16:57Z

Cus yolo.
I have no idea how much that stuff matters to the repo size, so best not to prematurely optimize.

TobiKattmann added 6 commits December 10, 2020 20:39

Remove unused SetMeshFile function.

aa14cf8

Remove deprecated Conig options WRT_LIMITERS/RESIDUALS.

956c678

Remove WRT_CON_FREQ_DUALTIME option. Only used in legacy output.

80dbb7f

Some additions to the config_template.

0d24af0

Add rank to volume output of (in-)compressible and heat solver.

d008136

Happy little fixes.

1bd0252

pr-triage bot added the PR: unreviewed label Dec 10, 2020

TobiKattmann mentioned this pull request Dec 10, 2020

Remove deprecated option WRT_CON_FREQ_DUALTIME. su2code/Tutorials#5

Merged

Replace WRT_CON_FREQ_DUALTIME with SCREEN_WRT_FREQ_TIME in legacy out…

b434fc6

…put.

pcarruscag added 3 commits December 11, 2020 11:46

WRT_SURF_FREQ_DUALTIME

76c0fef

WRT_OUTPUT

bc55314

WRT_VOL_SOL

8fa6238

TobiKattmann and others added 7 commits December 11, 2020 13:05

Deprecate WRT_SOL_FREQ(_DUALTIME). OUTPUT_WRT_FREQ replaced both.

37350d4

Merge remote-tracking branch 'origin/chore_minor_cleanups' into chore…

48e7fbe

…_minor_cleanups

WRT_SRF_SOL WRT_CSV_SOL WRT_CRD_SOL

6d80751

Merge branch 'chore_minor_cleanups' of https://github.com/su2code/SU2 …

2ccd429

…into chore_minor_cleanups

fix merge

5b4ce10

copy paste typos

c90efac

Merge remote-tracking branch 'upstream/develop' into chore_minor_clea…

67a7a60

…nups

WRT_HALO WRT_SHARPEDGES

1240a47

delete unused variables, QCR duplication, fix fsi regression

1f977f7

pcarruscag added the changelog:chore label Dec 11, 2020

pr-triage bot added PR: reviewed-approved and removed PR: unreviewed labels Dec 11, 2020

Merge remote-tracking branch 'origin/develop' into chore_minor_cleanups

b13e10b

pr-triage bot added PR: unreviewed and removed PR: reviewed-approved labels Dec 12, 2020

TobiKattmann added 2 commits December 12, 2020 23:28

Revert "try harder"

834cc1f

This reverts commit d8dcfb0.

Remove WRT_CON_FREQ once again without rewriting files

8100892

pcarruscag reviewed Dec 12, 2020

View reviewed changes

TestCases/hom_euler/NACA0012_5thOrder/fem_NACA0012.cfg Outdated Show resolved Hide resolved

TobiKattmann added 3 commits December 13, 2020 00:34

Deprecate WRT_SURFACE config_options.

fd191aa

Remove text above removed/deprecated WRT_CON_FREQ option.

acb8171

Adapt Quickstart. Remove completely unused Wrt_Slice variable from Co…

4b847e2

…nfig.

TobiKattmann commented Dec 13, 2020

View reviewed changes

pcarruscag reviewed Dec 13, 2020

View reviewed changes

SU2_CFD/src/output/output_structure_legacy.cpp Outdated Show resolved Hide resolved

pcarruscag changed the title ~~Minor cleanups / Remove deprecated options~~ Deprecate options that did not work since v7.0.0, and other minor cleanups Dec 13, 2020

TobiKattmann added 3 commits December 13, 2020 01:52

Re-add compatible options into legacy output for deprecated options.

18c4ea7

fix builds

8705735

Remove WRT_SURFACE from py-scripts to (hopefully) fix the regresion t…

1b6fcc3

…ests.

Merge remote-tracking branch 'origin/develop' into chore_minor_cleanups

efa8f13

pcarruscag merged commit 2250751 into develop Dec 14, 2020

pcarruscag deleted the chore_minor_cleanups branch December 14, 2020 22:31

pr-triage bot added PR: merged and removed PR: unreviewed labels Dec 14, 2020

TobiKattmann mentioned this pull request Dec 16, 2020

Dynamic FSI Adjoint #868

Closed

5 tasks

TobiKattmann mentioned this pull request Mar 18, 2021

Fixed values for turbulence quantities in upstream half-plane #1236

Merged

4 tasks

Conversation

TobiKattmann commented Dec 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed Changes

Related Work

PR Checklist

Uh oh!

pcarruscag commented Dec 11, 2020

Uh oh!

TobiKattmann commented Dec 11, 2020

Uh oh!

pcarruscag commented Dec 11, 2020

Uh oh!

TobiKattmann commented Dec 11, 2020

Uh oh!

TobiKattmann commented Dec 11, 2020

Uh oh!

pcarruscag commented Dec 11, 2020

Uh oh!

TobiKattmann commented Dec 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pcarruscag commented Dec 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TobiKattmann commented Dec 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TobiKattmann commented Dec 11, 2020

Uh oh!

Uh oh!

TobiKattmann Dec 13, 2020

Choose a reason for hiding this comment

Uh oh!

TobiKattmann Dec 13, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

TobiKattmann commented Dec 13, 2020

Uh oh!

pcarruscag commented Dec 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TobiKattmann commented Dec 14, 2020

Uh oh!

pcarruscag commented Dec 14, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

TobiKattmann commented Dec 10, 2020 •

edited

Loading

TobiKattmann commented Dec 11, 2020 •

edited

Loading

pcarruscag commented Dec 11, 2020 •

edited

Loading

TobiKattmann commented Dec 11, 2020 •

edited

Loading

pcarruscag commented Dec 14, 2020 •

edited

Loading