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

Added computation of pressure gradient effects to transition model #957

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

wilsonrvornlgov
Copy link
Contributor

@wilsonrvornlgov wilsonrvornlgov commented Mar 20, 2022

This is the final merge request for the incremental implementation of the Gamma correlation-based transition model of Menter (2015) into the Nalu-Wind flow solver. The commits in this PR add a pressure gradient term [dV/dy, Eq. (11), Menter, 2015] to the transition correlation functions lambda0 & Re0c for turbulence transition predictions. There are several gradient and dot product operations that are required for this term. The additional gradient operators are added in the 'ngp_algorithms' subdirectory and are computed in the GammaEquationSystem.C module.

Test cases for transition prediction have been added to the reg_tests directory for the ERCOFTAC T3A flat plate & Eppler airfoil. In addition, a repo for a 3D test case (6:1 ratio prolate spheroid geometries) will be created along with experimental results and predictions from other codes for comparison.

Pull-request type:

  • Bug fix

  • Documentation update

  • Feature enhancement

  • Builds successfully (must test on at least one system/compiler combination)

    • Operating systems
      • Linux
      • MacOS
    • Compilers
      • GCC
      • LLVM/Clang
      • Intel compilers
      • NVIDIA CUDA
  • Compiles without warnings

  • Passes all unit tests

  • Passes all regression tests

  • Documentation builds without errors

For new code updates

  • Documentation updates - additions to user, theory, and verification manuals
  • New unit tests providing coverage for new code, bug fixes etc.
  • New regression tests exercising the new code

@wilsonrvornlgov
Copy link
Contributor Author

wilsonrvornlgov commented Mar 22, 2022

@psakievich @jamelvin @marchdf The mesh for the Spheroid test case is 109MB, but the Github file size limit appears to be 100MB. Do you have any suggestions for a workaround to this limit?

@alanw0
Copy link
Contributor

alanw0 commented Mar 22, 2022

Why put in such a large test case? Test cases should contain relevant features and complexities, but not be huge in size.

@wilsonrvornlgov
Copy link
Contributor Author

wilsonrvornlgov commented Mar 22, 2022

Why put in such a large test case? Test cases should contain relevant features and complexities, but not be huge in size.

@alanw0 This would not be part of the test cases in reg_tests. I created a separate stand alone repo on GitHub to hold 3D test case files and document results from transition model development. The purpose is to show model capability for a 3D test case as part of closing this activity.

I have other 2D/smaller test cases in reg_tests to be used more routinely.

@psakievich
Copy link
Contributor

@wilsonrvornlgov how did you generate the mesh? Could we upload a script for generating the mesh instead of the mesh itself?

@wilsonrvornlgov
Copy link
Contributor Author

wilsonrvornlgov commented Mar 22, 2022

@wilsonrvornlgov how did you generate the mesh? Could we upload a script for generating the mesh instead of the mesh itself?

@psakievich I generated the meshes with Pointwise. It was not that complex, but not simple enough to recreate with a script. I copied the 3 spheroid test case input and mesh files (angle of attack 5, 10, 15 degs) to a directory on Eagle (/projects/hfm/FULL_SPHEROID).

@psakievich
Copy link
Contributor

@wilsonrvornlgov that seems good enough for now. thanks for doing that. I'll take a look at the code you've added

src/GammaEquationSystem.C Outdated Show resolved Hide resolved
Copy link
Contributor

@psakievich psakievich left a comment

Choose a reason for hiding this comment

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

@wilsonrvornlgov I feel like I'm missing something so it could be something subtle. What is the difference between the NDotVGradAlg computations you added and the NodalGradElem and NodalGradEdge algorithms that already exist?

src/ngp_algorithms/WallDistGradAlgDriver.C Outdated Show resolved Hide resolved
src/ngp_algorithms/NDotVGradAlgDriver.C Outdated Show resolved Hide resolved
@wilsonrvornlgov
Copy link
Contributor Author

@wilsonrvornlgov I feel like I'm missing something so it could be something subtle. What is the difference between the NDotVGradAlg computations you added and the NodalGradElem and NodalGradEdge algorithms that already exist?

@psakievich If i understand your question correctly, I think you are asking why I created 2 additional gradient operators for the WallDistGradAlg & NDotVGradAlg? For example, in GammaEquationSystem.C constructor (lines 123-125), I did this:

nodalGradAlgDriver_(realm_, "dgamdx"),
walldistGradAlgDriver_(realm_, "dWallDistdx"),
ndotvGradAlgDriver_(realm_, "dNDotVdx")

Instead of this:

nodalGradAlgDriver_(realm_, "dgamdx"),
nodalGradAlgDriver_(realm_, "dWallDistdx"),
nodalGradAlgDriver_(realm_, "dNDotVdx")

If so, Sayerhs told me the only way to compute the necessary terms was to duplicate the nodalGradAlgDriver for each gradient computation. Let me know if I misunderstood your question.

include/GammaEquationSystem.h Outdated Show resolved Hide resolved
@psakievich
Copy link
Contributor

@wilsonrvornlgov you didn't misunderstand my question. I don't think you need new classes though. Just new objects for each computation. See my suggested change.

Copy link
Contributor

@psakievich psakievich left a comment

Choose a reason for hiding this comment

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

@wilsonrvornlgov I approve of these new changes. Unfortunately we had a GPU diff and failures for the NREL5MW around your past commits and another one so we won't be able to merge until we get those sorted.

@psakievich psakievich added the wip-no-merge WIP, pull-request meant for discussions label Apr 11, 2022
@michaelasprague
Copy link
Contributor

@wilsonrvornlgov @psakievich Would you please comment on the latest status here and next steps for merge?

@psakievich
Copy link
Contributor

psakievich commented Jun 8, 2022

@michaelasprague some where in the previous two PR's a diff was introduced that was causing the turbine runs on summit to fail. We've reverted them for now. The next step is to re inspect those commits and fix the problem. Then this can be merged.

In terms of the timeline for that, I'm not sure. We should discuss the priority of this vs the other things going on. @PaulMullowney can you comment on how to reproduce the diff? My memory is a little hazy right now. It would be good to to document it in this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wip-no-merge WIP, pull-request meant for discussions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants