-
Notifications
You must be signed in to change notification settings - Fork 55
Add implicit diffusion for u and v #2688
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
Open
ewquon
wants to merge
79
commits into
erf-model:development
Choose a base branch
from
ewquon:implicit_diff_vel
base: development
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
by (1 - implicit_factor)
Multiply implicit soln by rho Use correct src/dst comp index after implicit solve
**BUGS FIXED** in ComputeStrain_S affecting tau13, tau23 compare with code before explicit_fac mods (e.g., commit 7c69e4c PR2643) * tau13 on z-lo w/o z-dirichlet: factor of 0.5 not multiplied through * tau23 on z-lo w/o z-dirichlet: factor of 0.5 not multiplied through * tau13 in interior: factor of mfx, mfy incorrectly multiplied through * tau23 in interior: factor of 0.5 not multiplied through
Contributor
Author
|
Added some verification notes (https://github.com/erf-model/ERF/blob/ab1590d292908a0f6ee473abe73891ffc68f8d50/Exec/ABL/column_diffusion_test/README.md#findings) In summary, for small dt (diffusive CFL < 0.5) the error between the fully explicit and mixed implicit-explicit paths is negligible. The solution is numerically stable with implicit diffusion for CFL > 5 but the velocity profile begins to drift. |
Do additional due diligence
ImplicitDiffForMom_T now more closely resembles _S detJ is only used for removing implicit_fac times the contribution to u,v from the d/dz components of the stress tensor
Note: in ComputeStrain, this is third-order at the boundaries; we can only do first-order for the tridiagonal solve.
202d031 to
cba52df
Compare
…on flags for testing
Co-authored-by: Mahesh Natarajan <[email protected]>
* NetCDF/RRTGMP/Particles CI * Remove commented out Spack commands Removed commented-out lines for spack view commands. * windows with ms-mpi attempt 1 * windows with ms-mpi attempt 1 * windows with ms-mpi attempt 2 * add option for MPI in cmake * fix test path * fix indentation * fix paths * Ctest with powershell? * Ctest with powershell 2 * Simplifying installing MS-MPI + more * Change shell and MPI executable path * MPI wrapper script * More MPI test changes (hopeful) * MS-MPI test * MS-MPI from correct source * MS-MPI from correct source 2 * MSI vs EXE * Exe nonewwindow * Remove exe * Remove exe 2 * Compile only, not run * Binary artifact * Binary artifact 2 * Whole build directory Updated artifact upload paths to include the entire build directory. * Modify Windows MPI workflow and enhance README Updated job configuration for Windows MPI workflow to support both OFF and ON variants for MPI. Added detailed installation instructions and troubleshooting steps in the README. * Style fixes * Remove non-MPI builds * Put regular windows.yml back to prior state --------- Co-authored-by: Aaron M. Lattanzi <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implicit diffusion options:
At this point, implicit vertical velocity appears to have marginal benefit and remains an experimental feature that hasn't been tested for terrain. This is enabled by the
-DERF_IMPLICIT_Wcompiler flag or theERF_ENABLE_IMPLICIT_Wcmake option.Verification problem for N and S paths:
Exec/ABL/column_diffusion_test/inputs_sfclay_pbl