Skip to content

Conversation

@Cristopher-Morales
Copy link
Contributor

@Cristopher-Morales Cristopher-Morales commented Apr 29, 2022

Proposed Changes

Give a brief overview of your contribution here in a few sentences.

Adapting and fixing mixing density model when species transport equations are being solved.
The species dependent properties will be added in 3 PRs:
1/3 : mixture density model (this PR)
2/3 : mixture viscosity + heat capacity
3/3 : mixture mass diffusivity + thermal conductivity

Related Work

Resolve any issues (bug fix or feature request), note any related PRs, or mention interactions with the work of others, if any.
Mark Heimgartner implemented the MIXTURE_FLUID model during his master thesis at Bosch. In this pull request, we aim to adapt his implementation to the latest version of develop branch, fixing some implementations that we think were causing numerical issues in his original implementation.

EDIT: Here, the mixture density will be implemented, based on the ideal gas law (computing mixture molecular weights).

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.

  • [X ] I am submitting my contribution to the develop branch.
  • [ X] 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).
  • [ X] 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.

@lgtm-com
Copy link

lgtm-com bot commented Apr 29, 2022

This pull request fixes 1 alert when merging c970543 into 6acc62f - view on LGTM.com

fixed alerts:

  • 1 for Dead code due to goto or break statement

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.

Looks good.

@lgtm-com
Copy link

lgtm-com bot commented Apr 30, 2022

This pull request fixes 1 alert when merging e8d75bc into 6acc62f - view on LGTM.com

fixed alerts:

  • 1 for Dead code due to goto or break statement

@bigfooted
Copy link
Contributor

@Cristopher-Morales do we really need all the diffusivity model implementations at this point (constant Schmidt etc)?
If the goal here is to push mixture density only, then these additional files can be in a separate pull request.

Cristopher-Morales and others added 6 commits May 10, 2022 09:23
Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>
Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>
@lgtm-com
Copy link

lgtm-com bot commented May 10, 2022

This pull request fixes 1 alert when merging 69ba724 into fa910a0 - view on LGTM.com

fixed alerts:

  • 1 for Dead code due to goto or break statement

@lgtm-com
Copy link

lgtm-com bot commented May 10, 2022

This pull request fixes 1 alert when merging f49c522 into fa910a0 - view on LGTM.com

fixed alerts:

  • 1 for Dead code due to goto or break statement

@pcarruscag pcarruscag changed the title Feature mixturedensity [WIP] Feature mixturedensity May 12, 2022
Cristopher-Morales and others added 3 commits May 13, 2022 09:55
Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>
Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>
Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>
%
FLUID_MODEL= FLUID_MIXTURE
%
MOLECULAR_WEIGHT= 28.96, 16.043
Copy link
Member

Choose a reason for hiding this comment

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

We need some comments here and in config_template regarding how this works, i.e. the "N+1" requirement and how they should be ordered.

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 will add them. Thank you so much for the feedback

@lgtm-com
Copy link

lgtm-com bot commented Jun 7, 2022

This pull request fixes 1 alert when merging a2f682e into 0232486 - view on LGTM.com

fixed alerts:

  • 1 for Dead code due to goto or break statement

@lgtm-com
Copy link

lgtm-com bot commented Jun 7, 2022

This pull request fixes 1 alert when merging 98c61f0 into 0232486 - view on LGTM.com

fixed alerts:

  • 1 for Dead code due to goto or break statement

@lgtm-com
Copy link

lgtm-com bot commented Jun 8, 2022

This pull request fixes 1 alert when merging 9dd6ea1 into c101e20 - view on LGTM.com

fixed alerts:

  • 1 for Dead code due to goto or break statement

@Cristopher-Morales
Copy link
Contributor Author

Hi all, thank you so much for your feedback. Are there things left for this pull request?
Thank you so much in advance.

@lgtm-com
Copy link

lgtm-com bot commented Jun 9, 2022

This pull request fixes 1 alert when merging 2b048b1 into c101e20 - view on LGTM.com

fixed alerts:

  • 1 for Dead code due to goto or break statement

Cristopher-Morales and others added 4 commits June 10, 2022 14:32
Co-authored-by: TobiKattmann <31306376+TobiKattmann@users.noreply.github.com>
Co-authored-by: TobiKattmann <31306376+TobiKattmann@users.noreply.github.com>
Co-authored-by: TobiKattmann <31306376+TobiKattmann@users.noreply.github.com>
Co-authored-by: TobiKattmann <31306376+TobiKattmann@users.noreply.github.com>
@lgtm-com
Copy link

lgtm-com bot commented Jun 10, 2022

This pull request fixes 1 alert when merging 4776aec into c101e20 - view on LGTM.com

fixed alerts:

  • 1 for Dead code due to goto or break statement

@lgtm-com
Copy link

lgtm-com bot commented Jun 13, 2022

This pull request fixes 1 alert when merging 0e404e3 into c101e20 - view on LGTM.com

fixed alerts:

  • 1 for Dead code due to goto or break statement

Copy link
Contributor

@TobiKattmann TobiKattmann left a comment

Choose a reason for hiding this comment

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

Hi Christopher, thanks for working on this and taking the responsibility to bring that contribution into develop 💐 (and of course working through all the comments, especially where some of mine were not correct 👍 )

Apart from these 2 more suggestions this PR is good to go from my point of view

Cristopher-Morales and others added 3 commits June 16, 2022 15:41
Co-authored-by: TobiKattmann <31306376+TobiKattmann@users.noreply.github.com>
…2_primitiveVenturi_mixingmodel.cfg

Co-authored-by: TobiKattmann <31306376+TobiKattmann@users.noreply.github.com>
@lgtm-com
Copy link

lgtm-com bot commented Jun 16, 2022

This pull request fixes 1 alert when merging 2c0d6c1 into db8ea45 - view on LGTM.com

fixed alerts:

  • 1 for Dead code due to goto or break statement

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