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

Refactor ConstantAlpha to allow running with turbulence model #1391

Merged
merged 16 commits into from
Jan 22, 2024

Conversation

ewquon
Copy link
Contributor

@ewquon ewquon commented Jan 19, 2024

Previous approach:

  • Calculate shear stress (tau), which in general is strain rate multiplied by 2*(mu + mu_t) -- here mu and mu_t refer to the molecular and turbulent viscosities
  • When calculating the diffusion contribution, multiply by rho/rho0 if using ConstantAlpha -- here, rho is the instantaneous local value, averaged from cell centers to faces
  • The issue arises because mu_t also ends up being scaled by this density ratio

New approach

  • Calculate shear stress with appropriate mu_eff, based on whether we're using the Constant or ConstantAlpha solver path -- here, tau is scaled by the cell-centered or face-averaged value of rho, depending on the stress quantity
  • Diffusion contribution calculations are agnostic to the molec_diff_type
  • Because the density used to scale the diffusion contribution is different, the gold data for regtests with ConstantAlpha will change

Tests based on DensityCurrent

  • ConstantAlpha with les_type=none: dynamicViscosity=75, rho0_trans=1 (default) vs dynamicViscosity=150, rho0_trans=2 -- verified results were identical
  • ConstantAlpha with les_type=Smagorinsky: dynamicViscosity=75, rho0_trans=1 (default) vs dynamicViscosity=150, rho0_trans=2 -- verified results were identical
  • very small differences observed with and without Smagorinsky in this case

ewquon and others added 8 commits January 19, 2024 15:17
- ComputeStress*() now have cell_data as an argument
- cell_data is set if l_use_constAlpha, otherwise a nullptr is passed
- mu_eff is now converted to alpha prior to being passed to
  ComputeStress*()
For generality, rho scaling is applied when tau is calculated. This
enables ConstantAlpha to be used with a turbulence model.
@ewquon
Copy link
Contributor Author

ewquon commented Jan 20, 2024

Density current prob (coarse dx=dz=100 m) at 900 s, before/after this PR
compare_before_after_PR1391

@ewquon ewquon changed the title Allow ConstantAlpha with turbulence model Refactor ConstantAlpha to allow running with turbulence model Jan 20, 2024
Copy link
Collaborator

@AMLattanzi AMLattanzi left a comment

Choose a reason for hiding this comment

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

Fairly certain the cell_data must be averaged to the strain locations (as is done in DiffusionSrcForState_N/T.cpp. Once that is completed, I am good with this PR.

},
[=] AMREX_GPU_DEVICE (int i, int j, int k) noexcept {
tau23(i,j,k) *= -cell_data(i, j, k, Rho_comp) * mu_eff;
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

For the off-diagonal stresses, shouldn't the cell_data be averaged to the location of the strain (this would hold for other places in the code as well)?
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, you're right. I have a fix coming.

@ewquon
Copy link
Contributor Author

ewquon commented Jan 22, 2024

@AMLattanzi, 23d55a5 fixes the segfault that showed up in the CI and 1834373 does the averaging that's needed.

@ewquon
Copy link
Contributor Author

ewquon commented Jan 22, 2024

Actually hold on, I only updated the terrain solver path.

@ewquon
Copy link
Contributor Author

ewquon commented Jan 22, 2024

Ok, @AMLattanzi, this latest commit makes the rest of the changes. I've verified that the rho averaging for the ConstantAlpha path matches the original mu_turb averaging.

Otherwise ref solution crashes on step 938 (t=234.25). Orig fixed dt ratio was
4; allowing a variable dt ratio results in a constant dt ratio of 6.
@ewquon ewquon enabled auto-merge (squash) January 22, 2024 18:34
@ewquon ewquon merged commit 9ea9813 into erf-model:development Jan 22, 2024
11 checks passed
@ewquon ewquon deleted the constalpha_with_turb branch January 22, 2024 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants