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

Species transport solver (passive for now) #1388

Merged
merged 139 commits into from
Dec 1, 2021
Merged

Conversation

TobiKattmann
Copy link
Contributor

@TobiKattmann TobiKattmann commented Oct 2, 2021

Proposed Changes

Hi all,

This PR implements a passive species transport solver. The passive means, that the mass fraction at each location does not influence any fluid-properties and thereby the mean flow (which reminds me that this is a good sanity check to compare mean flow residuals).

A new CSpeciesSolver and CSpeciesVariable were added based on the CScalar* things done in #1330 . Additionally species_convection/diffusion/sources were added just as it is the case for the turbulence solver (both based on the general scalar* framework)

Please note that prob ~1000 lines are cfg's, README's, helping scripts.

Related Work

Preliminary work was done in #1330 with creating a base class for scalar transport equations.

#1223 and #1332 already mostly implement the here shown species solver and big chunks were copied over from there, but they are not based on the new CScalarSolver. In order to chunk the work for these branches this PR will not contain any species mixing properties or TableLookUps.

After this PR #1332 will be tackled next. Whether we try to merge develop then into those branches or copy stuff over in a new branch will be seen when we get there.

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.

Note that sources are empty for now. Axissymmetric source terms will be
added later.
Right now, checking of these options is missing. Options also have to be
added to the config_template once everything else is finalized.
Please note that Multizonedriver is not tested yet. Single and
Multizonedriver get SetInitialCondition function which might be
superfluous - needs checking.
The output should be able to handle arbitrary many species and should
prob be generalized for comp and inc solvers to avoid code duplication.
@TobiKattmann TobiKattmann changed the title Species transport solver, [WIP] Species transport solver (passive for now) Oct 2, 2021
@pr-triage pr-triage bot removed the PR: unreviewed label Oct 2, 2021
@TobiKattmann TobiKattmann marked this pull request as draft October 2, 2021 09:27
@pr-triage pr-triage bot added the PR: draft label Oct 2, 2021
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.

That was quick 🥇

Common/include/CConfig.hpp Outdated Show resolved Hide resolved
Common/include/CConfig.hpp Outdated Show resolved Hide resolved
Common/include/CConfig.hpp Outdated Show resolved Hide resolved
Common/include/CConfig.hpp Outdated Show resolved Hide resolved
Common/include/CConfig.hpp Show resolved Hide resolved
SU2_CFD/src/solvers/CSpeciesSolver.cpp Outdated Show resolved Hide resolved
SU2_CFD/src/solvers/CSpeciesSolver.cpp Outdated Show resolved Hide resolved
SU2_CFD/src/solvers/CSpeciesSolver.cpp Outdated Show resolved Hide resolved
SU2_CFD/src/solvers/CSpeciesSolver.cpp Outdated Show resolved Hide resolved
SU2_CFD/include/variables/CSpeciesVariable.hpp Outdated Show resolved Hide resolved
Common/src/CConfig.cpp Outdated Show resolved Hide resolved
TobiKattmann and others added 2 commits October 3, 2021 11:26
Deleting empty destructors
Const-ing some code parts
OMP ready container positions in CDriver

Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>
Removing unnecessary (void)
Const-ing some return pointers
Renamming enum class entry NO_SCALAR_MODEL to NONE
@su2code su2code deleted a comment from lgtm-com bot Nov 29, 2021
@su2code su2code deleted a comment from lgtm-com bot Nov 29, 2021
@TobiKattmann
Copy link
Contributor Author

Alright, I think I am finished with this PR. Big thanks so far pedro, for all the comments, help, code suggestions and contributions 🤝 In case you have some final comments let me know

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.

Just minor comments below on the recent changes, very nice addition 🥇

SU2_CFD/include/output/CFlowOutput.hpp Outdated Show resolved Hide resolved
SU2_CFD/src/output/CFlowOutput.cpp Outdated Show resolved Hide resolved
species3_primitiveVenturi.test_vals = [-6.028145, -5.258104, -5.107927, -5.922051, -1.582604, -14.539029, -14.809699, 5, -0.808615, 5, -2.351160, 5, -0.288300, 1.645644, 0.499064, 0.601230, 0.545351]
species3_primitiveVenturi.test_vals = [-6.028145, -5.258104, -5.107927, -5.922051, -1.582604, -6.314220, -6.431771, 5, -0.808615, 5, -2.351160, 5, -0.288300, 1.645644, 0.499064, 0.601230, 0.545351]
Copy link
Member

Choose a reason for hiding this comment

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

Is any of the tests using the compressible solver?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, I don't have one. Tbh i never even ran a compressible case 🙊 I'll give it a try and report

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok i adatpted the rans/flatplate_SA to include some species inlet (with manually adapted inlet file to include something funny to happen). 0d37b2a I did not adapt the cfg too crazy to stay close to the original one. That is a task for another day

SU2_CFD/src/output/CFlowOutput.cpp Outdated Show resolved Hide resolved
SU2_CFD/src/output/CFlowOutput.cpp Outdated Show resolved Hide resolved
SU2_CFD/src/output/CFlowOutput.cpp Outdated Show resolved Hide resolved
SU2_CFD/src/output/CFlowOutput.cpp Outdated Show resolved Hide resolved
@@ -641,96 +756,13 @@ void CFlowOutput::SetAnalyzeSurface(const CSolver* const*solver, const CGeometry
}

/*--- Set values on markers ---*/
for (iMarker_Analyze = 0; iMarker_Analyze < nMarker_Analyze; iMarker_Analyze++) {
for (unsigned short iMarker_Analyze = 0; iMarker_Analyze < nMarker_Analyze; iMarker_Analyze++) {
su2double SpeciesVariance = Surface_SpeciesVariance_Total[iMarker_Analyze];
SetHistoryOutputPerSurfaceValue("SURFACE_SPECIES_VARIANCE", SpeciesVariance, iMarker_Analyze);
Tot_Surface_SpeciesVariance += SpeciesVariance;
// Set value into config. Necessary to access as an OF.
Copy link
Member

Choose a reason for hiding this comment

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

It's actually not needed, you can get it from the output, check if you can make the config const please.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am currently following the template which is used to set the OF for the DA. I actually used the Config to set the variance OF for this #1388 (comment) below. So I keep that in mind for the adjoint PR that will come next

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I would postpone this thought to the species adjoint PR

Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>
@TobiKattmann
Copy link
Contributor Author

TobiKattmann commented Nov 30, 2021

I also just made a gradient validation with the species variance OF ... will have the small additions in a follow up PR.
Small and easy case but with SST and 2 species transport equations

SURFACE_SPECIES_VARIANCE gradient 
+-----------+-------------------+-------------------+-------------------+-------------------+-------------+
| DV number |       DA gradient |       FD gradient |     absolute diff | relative diff [%] | sign change |
+-----------+-------------------+-------------------+-------------------+-------------------+-------------+
|         0 |     -0.0015565547 |     -0.0015568653 |      0.0000003106 |      0.0199523955 |           0 |
|         1 |     -0.0002704356 |     -0.0002704940 |      0.0000000584 |      0.0215893397 |           0 |
|         2 |      0.0005546628 |      0.0005546735 |     -0.0000000107 |     -0.0019320096 |           0 |
|         3 |      0.0008743823 |      0.0008743497 |      0.0000000325 |      0.0037214309 |           0 |
|         4 |      0.0008629093 |      0.0008627899 |      0.0000001194 |      0.0138354969 |           0 |
|         5 |     -0.0015565547 |     -0.0015568653 |      0.0000003106 |      0.0199522614 |           0 |
|         6 |     -0.0002704356 |     -0.0002704940 |      0.0000000584 |      0.0215901515 |           0 |
|         7 |      0.0005546628 |      0.0005546735 |     -0.0000000107 |     -0.0019318288 |           0 |
|         8 |      0.0008743823 |      0.0008743497 |      0.0000000325 |      0.0037199429 |           0 |
|         9 |      0.0008629093 |      0.0008627899 |      0.0000001194 |      0.0138353462 |           0 |
+-----------+-------------------+-------------------+-------------------+-------------------+-------------+

I only consider the y-direction of the points and and fixed the first two "columns" to have a smooth transition. Additionally during deformation i have the outlet as MARKER_SYM for smooth Volume mesh morphing , i.e. only the wall is in DV_MARKER. This Gradient validation was brought to you by: FADO :)
image

@su2code su2code deleted a comment from lgtm-com bot Nov 30, 2021
@su2code su2code deleted a comment from lgtm-com bot Nov 30, 2021
@su2code su2code deleted a comment from lgtm-com bot Dec 1, 2021
@su2code su2code deleted a comment from lgtm-com bot Dec 1, 2021
@lgtm-com
Copy link

lgtm-com bot commented Dec 1, 2021

This pull request introduces 1 alert and fixes 2 when merging d7757db into 42dcf5a - view on LGTM.com

new alerts:

  • 1 for Resource not released in destructor

fixed alerts:

  • 2 for Resource not released in destructor

@su2code su2code deleted a comment from lgtm-com bot Dec 1, 2021
@TobiKattmann TobiKattmann merged commit 1249f9f into develop Dec 1, 2021
@TobiKattmann TobiKattmann deleted the feature_species branch December 1, 2021 13:06
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.

3 participants