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

[WIP] Pressure Based Flow Solver V8 #2210

Draft
wants to merge 10 commits into
base: develop
Choose a base branch
from
Draft

Conversation

NAnand-TUD
Copy link

Proposed Changes

Give a brief overview of your contribution here in a few sentences.
Reimplemented pressure-based solver from feature_Pressure_based into the development branch.

Related Work

branch: feature_Pressure_based

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.
  • My contribution generates no new compiler warnings (try with --warnlevel=3 when using meson).
  • [ X] My contribution is commented and consistent with SU2 style (https://su2code.github.io/docs_v7/Style-Guide/).
  • I used the pre-commit hook to prevent dirty commits and used pre-commit run --all to format old commits.
  • 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.

Euler Solver: Incompressible cylinder case Pressure based (PB) vs Density Based (DB)
incomp_cylinder
incomp_cylinder_history

Navier Stokes Solver: Incompressible Hydrofoil case Pressure-based (PB) vs Density Based (DB)
NACA0012_hydrofoil
NACA0012_hydrofoil_history

Still TODOs

  • Fixing RANS: Current CIncPBVariable inherits CVariable instead of CFlowVariable, if this is fixed RANS should work.
  • Fixing MPI: The problem is currently unknown
  • Fixing ND: non-dimensionalization is different in the two solvers. It should be harmonized.
  • Multigrid: A PB multigrid is available but has not been tested.
  • Small style changes.

@NAnand-TUD NAnand-TUD requested review from pcarruscag, bigfooted and TobiKattmann and removed request for pcarruscag and bigfooted February 14, 2024 08:13
@NAnand-TUD NAnand-TUD marked this pull request as draft February 14, 2024 08:13
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

@jblueh
Copy link
Contributor

jblueh commented Feb 14, 2024

Target branch is master, you probably wanted it to be develop?

@bigfooted bigfooted changed the title Pressure Based Flow Solver V8 [WIP] Pressure Based Flow Solver V8 Feb 14, 2024
@bigfooted bigfooted changed the base branch from master to develop February 14, 2024 16:09
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.

Oof... I hope you have time because there's wayyyy too much duplicate code here.

@pcarruscag
Copy link
Member

But kudos for bringing this up to speed of course.
Maybe to start you and other reviewers can try to point out what aspects of this solver we can take from IncEuler and IncNs and the base flow solver.

@NAnand-TUD
Copy link
Author

Yes, I plan to invest some time here and there to finish this, but, not everything here falls under my expertise. I am currently making CPBIncEulerVariable to inherit CFlowVariable (instead of CVariable). This should already remove some duplication.

@bigfooted
Copy link
Contributor

And let's throw out everything that is not related to the PB solver.

@bigfooted
Copy link
Contributor

I pointed the PB solver to the existing vorticity and strain mag. functions and removed the heat.cpp file. If you can make your testcase available, then we can already setup our own regression tests.

@NAnand-TUD
Copy link
Author

Test cases added,

@NAnand-TUD
Copy link
Author

@bigfooted : I fixed the CFlowVariable thing. Many CPBIncEulerVariables are now commented out. CPBIncEulerSolver has the same behaviour as before now, however, PBIncNVSolver is crashing due to the initialization of vorticity. Maybe you can fix that.

@NAnand-TUD
Copy link
Author

NSSolver Vorticity error fixed. RANS Solver starts without crashing, results yet to be benchmarked.

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.

5 participants