Skip to content

Conversation

@mheimgartner
Copy link

@mheimgartner mheimgartner commented Jul 23, 2021

Proposed Changes

Hi all,

This draft PR contains all work on composition dependent fluid properties for multicomponent flow.

What I did so far:

• Implemented new CFluidScalar class (which inherits from CFluidModel) and can be accessed via a new fluid model: MIXTURE_FLUID_MODEL.
• Verified the current implementation by comparing with Fluent.
• Added regression test case for two component incomp_rans flow with variable fluid properties.

Future work:

• Fix variable specific heat capacity and convergence issues in case of axisymmetric flow.
• Add possibility to have more than a single transported scalar (i.e. mixture containing more than two components).
• Remove flamelet implementation from feature_multicomp in order to have this work self-contained. Note: Currently this is a PR into feature_flamelet to make the diff easier

In case there are comments, ideas, foreseeable stepping stones from experience please let me know.

Related Work

#1223 by @danielmayer @bigfooted which feature_multicomp currently branches from.
#1330 by @TobiKattmann which cuts the scalar transport necessary for this work out to merge it separately into develop

PR Checklist

Put an X by all that apply. You can fill this out after submitting the PR. If you have any questions, don't hesitate to ask! We want to help. These are a guide for you to know what the reviewers will be looking for in your contribution.

  • 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.

@pr-triage pr-triage bot added the PR: draft label Jul 23, 2021
@TobiKattmann
Copy link
Contributor

@mheimgartner first of all congrats to this openend PR

For the builds in the tests, compiler warnings are treated like errors. I.e. the code compiles and works on your side but the test-builds won't if certain warnings are present (like unused variables).
For more information where to look, see the Details button next to the individual builds. Click and wait a moment ... that will automatically scroll down for you.
The failing builds will be for the same code bits probably, so looking at BaseNoMPI for the start should suffice.

To look in the Details and change code from there might be tedious so you can see the warnings locally by setting ./meson.py <your build folder> ... --warnlevel=3 .... Work through the warnings until there are none left -> commit the changes and push. That will probably calm the compiler-warnings-are-errors gods and grant you access to the regression test

@WallyMaier
Copy link
Contributor

Hi @mheimgartner, thanks for the PR. I think this is a good use of the fluidmodel classes. There seems to be a lot of potential to generalize some aspects of Mixture_Fluid_Model and CNEMOGas. Im not sure what the best plan to approach this may be, but I think its something worth discussing.

@bigfooted
Copy link
Contributor

Hi @mheimgartner, thanks for the PR. I think this is a good use of the fluidmodel classes. There seems to be a lot of potential to generalize some aspects of Mixture_Fluid_Model and CNEMOGas. Im not sure what the best plan to approach this may be, but I think its something worth discussing.

@WallyMaier I think there might be some overlap in fluid properties like mixing models as well, but I do not know what information you have available in a library like mutation++. I assume that it has general fluid property calculations and mixing rules available that could be used (wilke rule, kinetic models, etc)?

@WallyMaier
Copy link
Contributor

Hi @mheimgartner, thanks for the PR. I think this is a good use of the fluidmodel classes. There seems to be a lot of potential to generalize some aspects of Mixture_Fluid_Model and CNEMOGas. Im not sure what the best plan to approach this may be, but I think its something worth discussing.

@WallyMaier I think there might be some overlap in fluid properties like mixing models as well, but I do not know what information you have available in a library like mutation++. I assume that it has general fluid property calculations and mixing rules available that could be used (wilke rule, kinetic models, etc)?

@bigfooted Thats correct! Both the CNEMOGas fluid models (SU2TCLib and CMutation++) have thermodynamic properties and transport properties. I imagine there would be a smart way to reduce overlap, though Im not sure what the best approach would be.

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.

Some comments below to keep in mind when you continue the implementation.

Comment on lines 30 to 31
unique_ptr<CViscosityModel> LaminarViscosityPointers[100];
unique_ptr<CConductivityModel> ThermalConductivityPointers[100];
Copy link
Member

Choose a reason for hiding this comment

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

either std::vector or std::array. If you go for a fixed size define it as a constexpr member variable.

Copy link
Author

Choose a reason for hiding this comment

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

Yes I hardcoded this because I don't know how to set the size corresponding to the number of species in the mixture.
Right now, I changed it to e.g.:
std::vector<std::unique_ptr> LaminarViscosityPointers;

and kept void CFluidScalar::SetLaminarViscosityModel(const CConfig* config) {} in CFluidScalar.cpp the same. Is this the correct way?


#include "CFluidModel.hpp"

class CFluidScalar : public CFluidModel {
Copy link
Member

Choose a reason for hiding this comment

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

missing override and or final specifiers in this class.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you Pedro for all your comments, really appreciate the help! Question regarding the CFluidScalar class which I made final, is it better to use override or final for its members, e.g.:

void SetLaminarViscosityModel(const CConfig* config) override;
vs
void SetLaminarViscosityModel(const CConfig* config) final;

If a member function is declared without virtual and with final, it means that it already overrides a virtual function in base class, so I figure it might be preferred to use final ? Also, do I need to state final after every function member in CFluidScalar?

wilkeNumerator.clear();
su2double viscosityMixture = 0.0;

massToMoleFractions(val_scalars);
Copy link
Member

Choose a reason for hiding this comment

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

This is wrong, too many hidden things happening, make the method const and the interface as explicit as possible.
x,y go in, z comes out.
It should not be necessary to read massToMoleFractions to understand wilkeViscosity

Copy link
Author

Choose a reason for hiding this comment

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

Right now I only moved massToMoleFractions to SetTDState_T. I did not make the method const because laminarViscosity is altered by wilkeViscosity. I placed laminarViscosity within the function because the laminarViscosity value may change at every grid point, e.g. when using Sutherland law in case of non-isothermal flow. Is there a better approach to do this?

…icomp

Conflicts:
	Common/include/CConfig.hpp
	Common/src/CConfig.cpp
	SU2_CFD/src/drivers/CDriver.cpp
	SU2_CFD/src/fluid/CFluidFlamelet.cpp
	SU2_CFD/src/numerics/CLookUpTable.cpp
	SU2_CFD/src/output/CFlowOutput.cpp
	SU2_CFD/src/output/output_structure_legacy.cpp
	SU2_CFD/src/solvers/CDiscAdjSolver.cpp
	SU2_CFD/src/solvers/CFlameletSolver.cpp
	SU2_CFD/src/solvers/CIncEulerSolver.cpp
	SU2_CFD/src/solvers/CPassiveScalarSolver.cpp
	SU2_CFD/src/solvers/CScalarSolver.cpp
@TobiKattmann
Copy link
Contributor

I hope to have fixed all regression testcases over at feature_flamelet so please merge and see whether we can get some ✔️'s here as well

TobiKattmann and others added 14 commits October 2, 2021 13:18
Had to add Linsol and Linsol_turb screen output to all of em.
See discussion 1403. Has to be implemented in a separate PR.
@TobiKattmann
Copy link
Contributor

The SST-2003 changes are in 33f9591 and were reverted directly afterwards in 8208c9e such that we have a commit with the necessary changes but they are not applied in the latest version of this code. This was done to revisit and finish this at a later stage.

Residual[iVar] += yinv*Volume*(Diffusion_Coeff_i[iVar]+Mass_Diffusivity_Tur)*ScalarVar_Grad_i[iVar][1];
else
Residual[iVar] += yinv*Volume*Density_i*(Diffusion_Coeff_i[iVar]+Mass_Diffusivity_Tur)*ScalarVar_Grad_i[iVar][1];
Residual[iVar] += yinv*Volume*(Density_i*Diffusion_Coeff_i[iVar]+Mass_Diffusivity_Tur)*ScalarVar_Grad_i[iVar][1];
Copy link
Contributor

Choose a reason for hiding this comment

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

This little bugfix in the axissymmetric source term let the corresponding reg test fail (which was to be expected). reg test-vals need to be adapted

TobiKattmann added a commit that referenced this pull request Oct 27, 2021
1. Consistent number of species vals defined per option. This setup will
see quite a bit more use with the multicomponent flow in #1332.
2. Boundness in [0,1] where applicable
@stale
Copy link

stale bot commented Mar 2, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is still a relevant issue please comment on it to restart the discussion. Thank you for your contributions.

@stale stale bot added the stale label Mar 2, 2022
@stale stale bot closed this Apr 16, 2022
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.

6 participants