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

Ww3 gse2 #1143

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

Ww3 gse2 #1143

wants to merge 13 commits into from

Conversation

aronroland
Copy link
Collaborator

Pull Request Summary

Description

Solves the anisotropic diffusion equation following Booij and Holthuijsen (1987, hereafter denoted as BH87). We apply a simple Galerkin scheme for the non-diagonal terms and treat the diagonal terms using linear shape functions. The scheme has been tested in various global setups. We have further developed the filter functions introduced by Hendrik, e.g., we fade out the diffusion coefficients when waves reach shallow water. This is crucial, as we have much higher resolution near the coastlines within the multiscale approach. An implicit implementation could also be a future option, but this, from my point of view, is not fruitful. For the interested reader, the justification for this is given by Peter Janssen in his JCP paper. Why use higher order schemes and then degrade them by adding diffusion? To keep it short, if the resolution goes to lim -> 0, the diffusion coefficient goes to infinity, which renders this approach not really optimal. We must keep in mind that all of this is only done since the model is limited by the finite bandwidth approximation, which was chosen to discretize the wave spectra. For example, when increasing the spectral resolution, the GSE effect vanishes. In conclusion, we can say that all this is not needed if the spectral resolution is high enough, which was not affordable at the time when this was developed. Nowadays, we must reevaluate the whole approach.

Please also include the following information:

  • Add any suggestions for a reviewer
    none
  • Mention any labels that should be added: bug, documentation, enhancement, new feature
    enhancement
  • Are answer changes expected from this PR? Please describe the changes and the reason why in addition to which of the following labels would apply: mod_def change, out_grd change, out_pnt change, restart file change, Regression test
    No!

Issue(s) addressed

#930

Commit Message

adding gse alleviation for ug grids.

Check list

  • [X ] Branch is up to date with the authoritative repository (NOAA-EMC) develop branch.
  • [ X] Checked the checklist for a developer submitting to develop.
  • If a version number update is required, checked the updating version number checklist.
  • [X ] If a new feature was added, a regression test for testing the new feature is added.
    This needs further attention by Jessica, we need to chose, which test we want.

Testing

  • How were these changes tested?
    Global runs with significant GSE.
  • Are the changes covered by regression tests? (If not, why? Do new tests need to be added?)
    Not yet see above.
  • Have the matrix regression tests been run (if yes, please note HPC and compiler)?
    Will be done by ERDC
  • Please indicate the expected changes in the regression test output, (Note the list of known non-identical tests.)
    None
  • Please provide the summary output of matrix.comp (matrix.Diff.txt, matrixCompFull.txt and matrixCompSummary.txt):
    Will follow ...

@JessicaMeixner-NOAA
Copy link
Collaborator

@aronroland thank you for submitting this PR! As soon as the testing information from ERDC is added we'll start the tests and review.

JGS_BLOCK_GAUSS_SEIDEL = .TRUE.
JGS_USE_JACOBI = .TRUE.
JGS_LGSE = .FALSE.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add descriptions of the new terms to model/inp/ww3_grid.inp in the UNST namelist section: https://github.com/NOAA-EMC/WW3/blob/develop/model/inp/ww3_grid.inp#L318

Copy link
Collaborator

Choose a reason for hiding this comment

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

@aronroland once this is done I can try to do some tests of this feature, but will need additional information before I can do so. We'll wait to run regression tests once we have testing information included in the PR. Lastly, you checked the box for "If a new feature was added, a regression test for testing the new feature is added", but I don't see the new regression test included yet, so we'll need that as well before this can be merged. That being said, as soon as I have enough information to run a test on my end I can do that.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for adding updates to ww3_grid.inp !

@aronroland aronroland marked this pull request as draft January 10, 2024 18:53
@aronroland
Copy link
Collaborator Author

aronroland commented Jan 10, 2024 via email

@JessicaMeixner-NOAA
Copy link
Collaborator

@aronroland Any updates on this?

@aronroland
Copy link
Collaborator Author

Dear @JessicaMeixner-NOAA,

we are writing the paper now on the GSE alleviation method we have implemented in WW3. As for the pull requests we need init_bugs2 1st before this goes in ...

@JessicaMeixner-NOAA
Copy link
Collaborator

@aronroland it seems like there are some outstanding issues with init_bugs2, can you help me understand why we cannot move forward with this PR? Is the other fixing a related bug?

@aronroland
Copy link
Collaborator Author

@thesser1 has commented on that and we r working actually on it. We should be soon done with the review of the changes.

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