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

Feature ddes #490

Merged
merged 95 commits into from
Jan 12, 2018
Merged

Feature ddes #490

merged 95 commits into from
Jan 12, 2018

Conversation

EduardoMolina
Copy link
Contributor

Dear SU2 developers,

This PR is a long effort to add Hybrid RANS/LES capabilities, specially the Delayed Detached Eddy Simulation with Grey area mitigation and low dissipation schemes proper to unsteady flows.

Summary of the contribution:

  • Delayed DES:

    • Standard SGS
    • Vorticity SGS
    • Shear Layer Adapted SGS
  • Several low dissipation functions:

    • DDES FD
    • Ducros' shock sensor
    • NTS adaptive
  • Variants of SA turbulence model:

    • Compressibility correction
    • Edwards and Chandra
    • QCR 2000 (Quadratic constitutive relation)
  • Low dissipation schemes:

    • Low dissipation Roe.
    • Low Mach Roe.
    • Simple low dissipation AUSM (SLAU and SLAU2)

I added only a small regression test for 2D DDES, travis can not handle even the smallest 3D test case that I have. Suggestions are welcome.

Best,

Eduardo

Eduardo Molina and others added 30 commits April 11, 2016 21:20
First commit of Hybrid RANS/LES
- Roe Low Dissipation: (1 - f_d) + Ducros Sensor
- Turbulence models: Edward’s SA, Compressibility Correction SA,
Edward’s + CC.
 - In SA_DDES, the low Reynolds term was disabled.
- Start of Venkatakrishnan Limiter Modification.
# Conflicts:
#	Common/include/config_structure.hpp
#	SU2_CFD/include/numerics_structure.hpp
#	SU2_CFD/src/SU2_CFD.cpp
#	SU2_CFD/src/driver_structure.cpp
#	SU2_CFD/src/numerics_direct_mean.cpp
# Resolved conflicts in:
#	Common/include/config_structure.hpp
#	SU2_CFD/include/numerics_structure.hpp
#	SU2_CFD/src/SU2_CFD.cpp
#	SU2_CFD/src/driver_structure.cpp
#	SU2_CFD/src/numerics_direct_mean.cpp
- Now ROW_LOW_DISSIPATION flag has the following options:
    - FD: Numerical blending using the DDES’s Fd function.
    - NTS: Numerical blending of Travin and Shur.
    - NTS_DUCROS: NTS + Ducros’ Shock Sensor.

All these options must be tested extensively.
- Update Ducros Sensor.
- Added a modified version of Venkatakrishnan Limiter.
Preventing the limiter to be greater than one.
It is incomplete right now.
- Add Weiss & Smith Preconditioner + Thornber Low Mach Correction
- Need to fix some conflicts.
Copy link
Member

@economon economon left a comment

Choose a reason for hiding this comment

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

Wow, long effort, indeed. Thanks @EduardoMolina! The community will really enjoy the new DES capabilities, low dissipation schemes, and S-A variants.

I've put a number of comments in the review on items to be cleaned up, but structurally, things look good so far. Can you also please check/remove the couple of Apple time machine files that are referenced in the change log?

.travis.yml Outdated
@@ -12,11 +12,12 @@ compiler:
notifications:
email:
recipients:
- [email protected]
- [email protected]
Copy link
Member

Choose a reason for hiding this comment

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

Don't forget to revert the .travis.yml file.

.travis.yml Outdated
@@ -80,7 +81,7 @@ install:

before_script:
# Get the test cases
- git clone -b develop https://github.com/su2code/TestCases.git ./TestData
- git clone -b feature_DDES https://github.com/su2code/TestCases.git ./TestData
Copy link
Member

Choose a reason for hiding this comment

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

Please also submit your mesh to the develop branch in the TestCases repo.


/*!
* \brief Get the Kind of Hybrid RANS/LES.
* \return Verbosity level for the console output.
Copy link
Member

Choose a reason for hiding this comment

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

Please update the "return" comments for these next few getter methods.

};
static const map<string, ENUM_LIMITER> Limiter_Map = CCreateMap<string, ENUM_LIMITER>
("NONE", NO_LIMITER)
("VENKATAKRISHNAN", VENKATAKRISHNAN)
("VENKATAKRISHNAN_WANG", VENKATAKRISHNAN_WANG)
("VENKATAKRISHNAN_2NDLIM", VENKATAKRISHNAN_2NDLIM)
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps this name is a little unclear..

@@ -161,7 +161,7 @@
05E6DBB817EB627400FA1F7E /* output_structure.hpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.h; lineEnding = 0; name = output_structure.hpp; path = ../../SU2_CFD/include/output_structure.hpp; sourceTree = "<group>"; xcLanguageSpecificationIdentifier = xcode.lang.cpp; };
05E6DBB917EB627400FA1F7E /* solver_structure.hpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.h; lineEnding = 0; name = solver_structure.hpp; path = ../../SU2_CFD/include/solver_structure.hpp; sourceTree = "<group>"; xcLanguageSpecificationIdentifier = xcode.lang.cpp; };
05E6DBBA17EB627400FA1F7E /* SU2_CFD.hpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.h; lineEnding = 0; name = SU2_CFD.hpp; path = ../../SU2_CFD/include/SU2_CFD.hpp; sourceTree = "<group>"; xcLanguageSpecificationIdentifier = xcode.lang.cpp; };
05E6DBBB17EB627400FA1F7E /* variable_structure.hpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.h; lineEnding = 0; name = variable_structure.hpp; path = ../../SU2_CFD/include/variable_structure.hpp; sourceTree = "<group>"; xcLanguageSpecificationIdentifier = xcode.lang.cpp; };
05E6DBBB17EB627400FA1F7E /* variable_structure.hpp */ = {isa = PBXFileReference; indentWidth = 2; lastKnownFileType = sourcecode.cpp.h; lineEnding = 0; name = variable_structure.hpp; path = ../../SU2_CFD/include/variable_structure.hpp; sourceTree = "<group>"; tabWidth = 2; xcLanguageSpecificationIdentifier = xcode.lang.cpp; };
Copy link
Member

Choose a reason for hiding this comment

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

Looks like the Xcode project had a small change. Try to revert if you can.


}

void CUpwRoe_Flow::ComputeResidual(su2double *val_residual, su2double **val_Jacobian_i, su2double **val_Jacobian_j, CConfig *config) {
Copy link
Member

Choose a reason for hiding this comment

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

Can you please check the changes in this file for the various upwind schemes? It seems to think that the entire subroutines were changed, when I think that the ordering or only white space changed. I suspect the true number of changed lines should be less.

/*--- L2Roe: a low dissipation version of Roe's approximate Riemann solver for low Mach numbers. IJNMF 2015 ---*/

zeta = min(1.0,max(Mach_i,Mach_j));
//zeta = max(zeta,0.05);
Copy link
Member

Choose a reason for hiding this comment

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

Remove this comment if unnecessary.


/*--- Evaluate Omega ---*/

//Omega = sqrt(Vorticity_i[0]*Vorticity_i[0] + Vorticity_i[1]*Vorticity_i[1] + Vorticity_i[2]*Vorticity_i[2]);
Copy link
Member

Choose a reason for hiding this comment

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

Remove commented line if unnecessary.

/*--- Production term ---*/;

// Original SA model
// Production = cb1*(1.0-ft2)*Shat*TurbVar_i[0]*Volume;
Copy link
Member

Choose a reason for hiding this comment

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

In general, please remove the commented lines from the original S-A model to keep the implementation as clean as possible (especially for new users).

AD::SetPreaccIn(Gradient_j, nPrimVarGrad, nDim);
AD::SetPreaccIn(Coord_i, nDim); AD::SetPreaccIn(Coord_j, nDim);

/*--- Simple and Parameter-Free Second Slope Limiter for Unstructured Grid Aerodynamic Simulations---*/
Copy link
Member

Choose a reason for hiding this comment

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

Having a parameter-free option is interesting.. folks still have trouble tuning limiters and avoiding limit cycles/stalled convergence. Have you had a positive experience with this limiter in terms of convergence?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a trick with this parameter-free second limiter. If you have a high quality 3D grid, this limiter will greatly improve convergence. However for complex geometries (most of the time in aerospace industry), this second limiter will diverge the solution because it will turn off the primary limiter in regions of poor quality cells when M<1.0.
There is a measure of the cell quality (Angle, Skewness, etc) in SU2? If we have, I would like to use here in order to avoid turning off the limiter in this regions.

Copy link
Member

Choose a reason for hiding this comment

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

I see some problems in the current way it is implemented. First of all doing a search involving neighbors shouldn't be done inside an edge loop as you compute the same values twice (just an efficiency thing). Furthermore, in order for the computation to give the same result using different number of processors (which we should aim for) a communication of the variable Mach_Local is necessary.

Unless you think that this limiter greatly improves convergence, I would suggest to wait with this limiter implementation until we have figured out a way of generalizing this.

@talbring
Copy link
Member

talbring commented Jan 8, 2018

Hm why are the regression tests failing ? It looks like is has something to do with the changes in the limiter in a recent commit.

@talbring
Copy link
Member

talbring commented Jan 8, 2018

Ok I think I found the problem, you accidentally removed the Set_MPI_Limiter call. Let's see what travis says.

Copy link
Member

@economon economon left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you for all of the effort. Time to merge this one...

@economon economon merged commit 61e7b90 into develop Jan 12, 2018
@economon economon deleted the feature_DDESv5.0 branch January 12, 2018 08:03
CatarinaGarbacz pushed a commit that referenced this pull request Aug 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants