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

Implement generalized advection schemes for RC NS implementation #20504

Merged
merged 11 commits into from Apr 26, 2022

Conversation

lindsayad
Copy link
Member

Closes #20493

@grmnptr if we want to make this happen, I'd appreciate your thoughts on how to best incorporate skew into this

@lindsayad lindsayad requested a review from GiudGiud as a code owner March 9, 2022 00:52
@moosebuild
Copy link
Contributor

moosebuild commented Mar 9, 2022

Job Documentation on 2648854 wanted to post the following:

View the site here

This comment will be updated on new commits.

@lindsayad lindsayad force-pushed the general-advection-schemes-20493 branch from ad3e8a9 to b0b6772 Compare March 24, 2022 19:55
@moosebuild
Copy link
Contributor

moosebuild commented Mar 25, 2022

Job Coverage on 2648854 wanted to post the following:

Framework coverage

4e0077 #20504 264885
Total Total +/- New
Rate 82.61% 82.62% +0.01% 85.39%
Hits 74484 74610 +126 228
Misses 15676 15691 +15 39

Diff coverage report

Full coverage report

Modules coverage

Navier stokes

4e0077 #20504 264885
Total Total +/- New
Rate 85.15% 85.29% +0.14% 92.86%
Hits 9647 9646 -1 143
Misses 1682 1663 -19 11

Diff coverage report

Full coverage report

Full coverage reports

Reports

Warnings

  • framework new line coverage rate 85.39% is less than the suggested 90.0%

This comment will be updated on new commits.

@lindsayad lindsayad force-pushed the general-advection-schemes-20493 branch 4 times, most recently from be84f84 to 07bd2b7 Compare March 27, 2022 21:08
@GiudGiud
Copy link
Contributor

The cool thing about using atomic interpolations at the inlet with Dirichlet BCs is that we no longer need flux BCs to do coupling with SAM.

@lindsayad lindsayad changed the title Implement generalized advection schemes for RC NS implementation [WIP] Implement generalized advection schemes for RC NS implementation Apr 4, 2022
@lindsayad lindsayad force-pushed the general-advection-schemes-20493 branch 2 times, most recently from 2b37c7f to e3cf758 Compare April 4, 2022 22:15
@lindsayad lindsayad changed the title [WIP] Implement generalized advection schemes for RC NS implementation Implement generalized advection schemes for RC NS implementation Apr 4, 2022
@lindsayad lindsayad marked this pull request as ready for review April 4, 2022 22:15
@lindsayad lindsayad force-pushed the general-advection-schemes-20493 branch 6 times, most recently from 8a977ee to 1f15e0c Compare April 7, 2022 00:11
Fits with the philosophy of put your code where you use it which the
new coverage testing encourages and which I love
- Have PINSFVMassAdvection inherit from INSFVMassAdvection
- Add epsilon virtual method to INSFVMomentumAdvection
@lindsayad lindsayad force-pushed the general-advection-schemes-20493 branch 5 times, most recently from f62ccec to 34ffd12 Compare April 26, 2022 02:30
…ights

This forces atomic evaluation of components of aggregate functors (such as
functor material properties), which allows direct substitution of Dirichlet
information when component(s) are variables. This allows more accurate setting
of incoming fluxes

This dramatically improves our results on skewed meshes, as demonstrated
on one of Sinan's cask setups

This prevents zero mixing lengths and zero eddy viscosity.

Additionally: change default for fv = true variable two-term boundary
expansion to `true`. If users select finite volume variables through
the `fv = true` flag as opposed to specifying `type = MooseVariableFVReal`,
then we need to make sure they get the same default for two term boundary
expansion, which is `true`.

Except when doing vertex based interpolation. This necessitated adding
a special handler in `Moose::FV::interpolate(FunctorBase, FaceArg)` as
the `Moose::FV::linearInterpolation(FunctorBase, FaceArg)` function is
the only global function that currently handles skewness correction
correctly.

I decided to make this change for MooseVariableFV::evaluate(FaceArg)
after trigging a debug mode assertion when interpolating passive scalars
in the 2d-mixing-length test in INSFV. When I had changed the scalar
field advection kernel to functor evaluation based off Face arguments,
then I actually changed the evaluation behind-the-scenes from upwind
for this test to central-differencing. This is because all other face
evaluations in INSFV (I think) go through functor material properties
and end up using the global `Moose::FV::interpolate` functions which apply
the correct interpolation/limiting method supplies by the user. However,
prior to this commit variable evaluations did their own handling without
dispatching to the global `Moose::FV::interpolate` functions and always
applied some form of central-differencing on internal faces. That is
now changed so the mixing length test is back to upwinding its passive scalar
transport, which necessitated another re-golding

- We make sure to extend the change in advection kernel evaluation at boundaries
  to the VolumetricFlowRate postprocessor.
- Evaluation of incoming momentum flux is more accurate with these changes which
  puts more tax on the wall-function tests. This is because before when we upwinded
  the momentum at the boundaries we would actually apply positive y-velocities
  at the incoming face near the wall which led to a non-monotone y-velocity
  profile near the wall moving from the inlet down the channel. Now we
  actually apply the 0 y-velocity at the inlet face which makes for a more
  challenging solve due to the large gradient in y-velocity the solver wants
  to impose due to the large viscous forces. (This is my hypothesis anyway). To
  make things easier on the solver, I've reduced the Reynolds number. Note that
  if you psuedo-step to steady-state, convergence is excellent with the original
  Reynolds number
- Created a couple of global functions for help in creating interpolation parameters
  and for using those parameters to set advection and velocity interpolation object
  data members
- Limiter/interpolation coverage enhancements
- An additional change in this commit is making PINSFVEnergyAdvection
  a trivial derivative of INSFVEnergyAdvection such that we reuse all
  the code, e.g. computeQpResidual
- Test vector and array functor material properties
- Allow construction of VectorCompositeFunctor with 1-3 functors
- Convert mixing length tests into transient tests

Refs joe61vette/ASP#8

temp
@lindsayad lindsayad force-pushed the general-advection-schemes-20493 branch from 34ffd12 to 2648854 Compare April 26, 2022 02:34
@lindsayad
Copy link
Member Author

Ok @GiudGiud I think this should be good again

Copy link
Contributor

@GiudGiud GiudGiud left a comment

Choose a reason for hiding this comment

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

excellent. I'm excited for all these changes! More robust wrt skewness, more user-friendly bcs, and no negative mixing length

@lindsayad lindsayad merged commit 47fd88c into idaholab:next Apr 26, 2022
FY22 NEAMS MP - TA Support automation moved this from In progress to Done Apr 26, 2022
@lindsayad lindsayad deleted the general-advection-schemes-20493 branch April 26, 2022 17:23
lindsayad added a commit to lindsayad/moose that referenced this pull request Apr 26, 2022
For higher-order advection schemes we need two layers. References
distributed mesh failures on next post idaholab#20504
lindsayad added a commit to lindsayad/moose that referenced this pull request Apr 27, 2022
lindsayad added a commit to lindsayad/moose that referenced this pull request May 3, 2022
grmnptr pushed a commit to grmnptr/moose that referenced this pull request May 3, 2022
For higher-order advection schemes we need two layers. References
distributed mesh failures on next post idaholab#20504
grmnptr pushed a commit to grmnptr/moose that referenced this pull request May 3, 2022
grmnptr pushed a commit to grmnptr/moose that referenced this pull request May 3, 2022
For higher-order advection schemes we need two layers. References
distributed mesh failures on next post idaholab#20504
grmnptr pushed a commit to grmnptr/moose that referenced this pull request May 3, 2022
lindsayad added a commit to lindsayad/moose that referenced this pull request May 3, 2022
Per Peter, QUICK is not TVD after rF=5 and SOU fails beyond rF=2.
Consequently, a coefficient of 1e6 is pretty ludicrous. Cutting to
1e3 seems to make nonlinear convergence quite a bit better for
a lid problem with symmetry in x-velocity about the vertical center-line.

Refs advection sheme test case introduced in idaholab#20504
lindsayad added a commit to lindsayad/moose that referenced this pull request May 3, 2022
Per Peter, QUICK is not TVD after rF=5 and SOU fails beyond rF=2.
Consequently, a coefficient of 1e6 is pretty ludicrous. Cutting to
1e3 seems to make nonlinear convergence quite a bit better for
a lid problem with symmetry in x-velocity about the vertical center-line.

Refs advection sheme test case introduced in idaholab#20504
lindsayad added a commit to lindsayad/moose that referenced this pull request May 10, 2022
Per Peter, QUICK is not TVD after rF=5 and SOU fails beyond rF=2.
Consequently, a coefficient of 1e6 is pretty ludicrous. Cutting to
1e3 seems to make nonlinear convergence quite a bit better for
a lid problem with symmetry in x-velocity about the vertical center-line.

Refs advection sheme test case introduced in idaholab#20504
lindsayad added a commit to lindsayad/moose that referenced this pull request May 10, 2022
Per Peter, QUICK is not TVD after rF=5 and SOU fails beyond rF=2.
Consequently, a coefficient of 1e6 is pretty ludicrous. Cutting to
1e3 seems to make nonlinear convergence quite a bit better for
a lid problem with symmetry in x-velocity about the vertical center-line.

Refs advection sheme test case introduced in idaholab#20504
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Enable advected schemes other than upwind/average for INSFV/WCNSFV
5 participants