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

Updating some of the NEMO viscous solver routines. #1347

Merged
merged 17 commits into from
Aug 21, 2021
Merged

Conversation

WallyMaier
Copy link
Contributor

@WallyMaier WallyMaier commented Aug 4, 2021

Proposed Changes

This PR updates some of the NEMO viscous routines to be more inline with the overall SU2 structure. Hopefully paving the way for an easier merge with turbulence solvers in the future.

Related Work

This is coming with an effort to update and optimize NEMO, like in #1343.

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 Aug 4, 2021
@lgtm-com
Copy link

lgtm-com bot commented Aug 4, 2021

This pull request fixes 1 alert when merging eab6dcd into f47e22b - view on LGTM.com

fixed alerts:

  • 1 for Comparison of narrow type with wide type in loop condition

@lgtm-com
Copy link

lgtm-com bot commented Aug 4, 2021

This pull request fixes 1 alert when merging 210fefc into f47e22b - view on LGTM.com

fixed alerts:

  • 1 for Comparison of narrow type with wide type in loop condition

@lgtm-com
Copy link

lgtm-com bot commented Aug 5, 2021

This pull request fixes 1 alert when merging aceb911 into f47e22b - view on LGTM.com

fixed alerts:

  • 1 for Comparison of narrow type with wide type in loop condition

@WallyMaier WallyMaier changed the title [WIP] Updating some of the NEMO viscous solver routines. Updating some of the NEMO viscous solver routines. Aug 5, 2021
@pcarruscag pcarruscag marked this pull request as ready for review August 7, 2021 08:44
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.

Thanks for the cleanup and modernization 👍, just a few comments below, one might be an issue (but easy to fix).

SU2_CFD/include/solvers/CFVMFlowSolverBase.hpp Outdated Show resolved Hide resolved
SU2_CFD/src/fluid/CNEMOGas.cpp Outdated Show resolved Hide resolved
SU2_CFD/src/solvers/CNEMONSSolver.cpp Outdated Show resolved Hide resolved
SU2_CFD/src/solvers/CNEMONSSolver.cpp Outdated Show resolved Hide resolved
SU2_CFD/src/solvers/CNEMONSSolver.cpp Outdated Show resolved Hide resolved
SU2_CFD/src/solvers/CNEMONSSolver.cpp Outdated Show resolved Hide resolved
SU2_CFD/src/solvers/CNEMONSSolver.cpp Outdated Show resolved Hide resolved
SU2_CFD/src/solvers/CNEMONSSolver.cpp Outdated Show resolved Hide resolved
SU2_CFD/src/solvers/CNEMONSSolver.cpp Outdated Show resolved Hide resolved
@pcarruscag pcarruscag changed the title Updating some of the NEMO viscous solver routines. [WIP] Updating some of the NEMO viscous solver routines. Aug 16, 2021
@WallyMaier WallyMaier changed the title [WIP] Updating some of the NEMO viscous solver routines. Updating some of the NEMO viscous solver routines. Aug 17, 2021
@lgtm-com
Copy link

lgtm-com bot commented Aug 17, 2021

This pull request fixes 1 alert when merging 9d086ab into 3ec1c68 - view on LGTM.com

fixed alerts:

  • 1 for Comparison of narrow type with wide type in loop condition

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.

Thanks Wally, you got some changes in codi and opdi by accident.
Revert those and this is good to go for me 👍

@pcarruscag
Copy link
Member

The versions you want are:
codi 3c3211fef2e225ab89680a4063b62bb3bb38a7e4
opdi 2735b503f60163e8d64e1ac56cce46173a9fd4a9

@WallyMaier
Copy link
Contributor Author

WallyMaier commented Aug 17, 2021

@pcarruscag Thats interesting, I just crossed check the versions in my init.py file and they match the versions you posted. Im not sure what how to revert these changes :(

@pcarruscag
Copy link
Member

checkout those commits I mentioned (by going inside externals/codi and externals/opdi) and then commit again and push

@lgtm-com
Copy link

lgtm-com bot commented Aug 17, 2021

This pull request fixes 1 alert when merging bc7aa7a into 3ec1c68 - view on LGTM.com

fixed alerts:

  • 1 for Comparison of narrow type with wide type in loop condition

@WallyMaier
Copy link
Contributor Author

Thanks! It looks like I needed to update a regression with change...strange. But its done.

@lgtm-com
Copy link

lgtm-com bot commented Aug 18, 2021

This pull request fixes 1 alert when merging 7a487f2 into 3ec1c68 - view on LGTM.com

fixed alerts:

  • 1 for Comparison of narrow type with wide type in loop condition

@@ -96,7 +96,7 @@ void CIncNSSolver::Preprocessing(CGeometry *geometry, CSolver **solver_container
SetPrimitive_Limiter(geometry, config);
}

ComputeVorticityAndStrainMag<1>(*config, iMesh);
Copy link
Contributor

Choose a reason for hiding this comment

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

What did the <1> here signify?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was part of a way to make a general function for all solvers. It is essentially VEL_INDEX

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.

None yet

3 participants