Fix dissipation in transition model and update inlet profile (initial profile from config)#1268

Merged

bigfooted merged 13 commits intodevelopfrom

fix_turbulence_input

Apr 21, 2021

Contributor

bigfooted commented Apr 18, 2021

Proposed Changes

fixed factor of 100 in the transition turbulence models
changed initialization of inlet profile from 0.0 to values taken from config file. I guess this was done to make this routine more general (for other boundaries) in the future. However, I think generating a default file that works out of the box is better.
added a descriptive comment line to the inlet profile file (one for each marker) with the names of the columns. This line is intended to make editing easier for the user. This comment line is not mandatory and is ignored by the profile reader.
do not write residuals of temperature to output file when energy equation is off

Related Work

Related to #1263 and #1265

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.

bigfooted added 3 commits

April 18, 2021 22:03


          inlet profile is now initialized from config options

131bc65


          Merge branch 'develop' into fix_turbulence_input

f713dd7


          fixed compiler warning

3d87e00

pr-triage bot added the PR: unreviewed label

pcarruscag requested changes

View reviewed changes

SU2_CFD/include/CMarkerProfileReaderFVM.hpp Show resolved Hide resolved

SU2_CFD/src/CMarkerProfileReaderFVM.cpp Outdated Show resolved Hide resolved

SU2_CFD/src/numerics/turbulent/turb_sources.cpp

-                    su2double tu = config->GetTurbulenceIntensity_FreeStream();
+                    /*--- turbulence intensity is u'/U so we multiply by 100 to get percentage ---*/
+                    su2double tu = 100.0 * config->GetTurbulenceIntensity_FreeStream();

Member

pcarruscag Apr 18, 2021

👍 every now and then CFD-online helps us find some bugs :)

pr-triage bot added PR: reviewed-changes-requested and removed PR: unreviewed labels


          changed implementation of numberOfTurbVars

f41c621

pr-triage bot added PR: unreviewed and removed PR: reviewed-changes-requested labels

pcarruscag reviewed

View reviewed changes

SU2_CFD/src/CMarkerProfileReaderFVM.cpp Outdated Show resolved Hide resolved

pcarruscag reviewed

View reviewed changes

SU2_CFD/include/CMarkerProfileReaderFVM.hpp Outdated Show resolved Hide resolved

pcarruscag reviewed

View reviewed changes

SU2_CFD/src/solvers/CSolver.cpp Outdated Show resolved Hide resolved

bigfooted and others added 7 commits

April 19, 2021 13:47


          Update SU2_CFD/include/CMarkerProfileReaderFVM.hpp

93c03dd

Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>


          updated codi/medi from develop

a58d609


          moved physics from profilereader to csolver

42c3bc8


          changed residuals of schubauer-klebanoff case

becd187


          change output profile file to example_filename

3c0dbc2


          changed residuals of schubauer-klebanoff parallel case

0bd2f7e


          Merge branch 'develop' into fix_turbulence_input

b67981f

pcarruscag approved these changes

View reviewed changes

SU2_CFD/src/solvers/CSolver.cpp Outdated Show resolved Hide resolved

SU2_CFD/src/solvers/CSolver.cpp Outdated

Comment on lines 3363 to 3364

		INLET_TYPE Kind_Inlet = config->GetKind_Inlet();
		switch (Kind_Inlet) {

Member

pcarruscag Apr 20, 2021

Suggested change

      
                  INLET_TYPE Kind_Inlet = config->GetKind_Inlet();
          
                  switch (Kind_Inlet) {
          
                  switch (config->GetKind_Inlet()) {

SU2_CFD/src/solvers/CSolver.cpp

Comment on lines +3359 to +3360

		columnName << "# COORD-X " << setw(24) << "COORD-Y " << setw(24);
		if(nDim==3) columnName << "COORD-Z " << setw(24);

Member

pcarruscag Apr 20, 2021

This is going to sound like neat picking in this particular context but bear with me (you never know the next piece of code you will work on).
From an efficiency standpoint you should always move as little data as possible (talking about the spaces in setw and passing numbers as strings).
This is particularly relevant for strings because of something called "small string optimization", where up to a certain number of characters, strings do not allocate memory on the heap.
In this example, this also means that you are forcing the file format from the outside, this should be the responsibility of the class (profile reader in this case) consider for example that we may want to use it for CSV formats.

SU2_CFD/src/solvers/CSolver.cpp Outdated

+                  columnName << "# COORD-X  " << setw(24) << "COORD-Y    " << setw(24);
+                  if(nDim==3) columnName << "COORD-Z    " << setw(24);
+                  if (compressible){

Member

pcarruscag Apr 20, 2021

This is ok for now, but if more solvers start using inlet profiles this is not a scalable solution.
The suggestion was to have this block of code as a method of different solvers, so that you do not have to figure out the type of solver based on configuration options (and consequently the type of inlet, etc. etc.).
solver->GetInletVariableNames() something like that.
The motivation for polymorphism and abstract classes is to allow centralizing this type of knowledge, i.e. "what type of object is this?", on a single place, the instantiation of the object.
Enough philosophy...

Contributor Author

bigfooted Apr 21, 2021

yes, I prefer having something like this at the level of the solver container so we can construct this information. After all, very soon we will have passive scalar, combustion variables, nemo combustion variables,...

pr-triage bot added PR: reviewed-approved and removed PR: unreviewed labels

pcarruscag added the changelog:feature label

Member

pcarruscag commented Apr 20, 2021

By the way please give the PR a more descriptive title that reflects the transition fix and the column names feature.
So that folks know what to expect from the change log.


          Update SU2_CFD/src/solvers/CSolver.cpp

0f3c962

Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>

pr-triage bot added PR: unreviewed and removed PR: reviewed-approved labels

bigfooted changed the title ~~Fix turbulence input~~ Fix dissipation in transition model and update inlet profile (initial profile from config)


          small fix due to merge #1264

dd0ba12

bigfooted merged commit 1c3610d into develop

bigfooted deleted the fix_turbulence_input branch

April 21, 2021 11:59

pr-triage bot added PR: merged and removed PR: unreviewed labels

This was referenced Apr 21, 2021

inlet profile from file is not initialized from config.cfg #1265

Closed

wrong turbulence intensity for transition models #1263

Closed

TobiKattmann reviewed

View reviewed changes

Contributor

TobiKattmann left a comment

A bit late, but thanks for the transition-model bugfix and the inlet-marker-stuff 💐

SU2_CFD/src/CMarkerProfileReaderFVM.cpp

Comment on lines 47 to +51

                 filename     = val_filename;
                 markerType   = val_kind_marker;
                 numberOfVars = val_number_vars;
+                columnNames  = val_columnNames;
+                columnValues = val_columnValues;

Contributor

TobiKattmann Apr 28, 2021 •

edited

Loading

I guess this could be done with an initializer list for the ctor .. but I am unsure if one is to be preferred over the other. The core guidelines dont seem to have a strong opinion either. CE Playground

SU2_CFD/src/CMarkerProfileReaderFVM.cpp

Comment on lines 130 to 134

    
                    }

                  } else {

                    SU2_MPI::Error("While opening profile file, no \"NMARK=\" specification was found", CURRENT_FUNCTION);

                    //cout << "inlet profile reader is ignoring line: " << text_line << endl;

                  }

                }

Contributor

TobiKattmann Apr 28, 2021

in that case I would remove the whole else statement ... unless there are further plans

SU2_CFD/src/output/CFlowIncOutput.cpp

Comment on lines 596 to +601

                   SetVolumeOutputValue("RES_VELOCITY-Z", iPoint, solver[FLOW_SOL]->LinSysRes(iPoint, 3));
-                  SetVolumeOutputValue("RES_TEMPERATURE", iPoint, solver[FLOW_SOL]->LinSysRes(iPoint, 4));
+                  if (config->GetEnergy_Equation())
+                    SetVolumeOutputValue("RES_TEMPERATURE", iPoint, solver[FLOW_SOL]->LinSysRes(iPoint, 4));
                 } else {
-                  SetVolumeOutputValue("RES_TEMPERATURE", iPoint, solver[FLOW_SOL]->LinSysRes(iPoint, 3));
+                  if (config->GetEnergy_Equation())
+                    SetVolumeOutputValue("RES_TEMPERATURE", iPoint, solver[FLOW_SOL]->LinSysRes(iPoint, 3));

Contributor

TobiKattmann Apr 28, 2021

I guess one could move this config->GetEnergy_Equation() out of the if nDim and make the acces solver[FLOW_SOL]->LinSysRes(iPoint, nDim+1)

pr-triage bot added PR: unreviewed and removed PR: merged labels

TobiKattmann added a commit that referenced this pull request


          Two remarks to PR #1268 of @bigfooted

f945eb1

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

Labels

changelog:feature PR: unreviewed