-
Notifications
You must be signed in to change notification settings - Fork 37
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
MALI calving dt convergence tests #386
MALI calving dt convergence tests #386
Conversation
Hello @matthewhoffman! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
Comment last updated at 2022-05-11 22:38:47 UTC |
@trhille , this is ready for review from you. I haven't added the docs yet, but the code changes are done and I thought you might want to start looking at this before I have the docs complete. To test this, you'll need to be using the accompanying MALI PR: MALI-Dev/E3SM#33. I'll update the submodule in this PR after we merge that. For testing, I recommend running the new (I'll wait to have Xylar review until I have the docs done.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@matthewhoffman, this is really impressive! I've finished going through the code (only found minor copy-paste errors and had a few questions), but I'm still running the tests.
compass/landice/tests/calving_dt_convergence/dt_convergence_test/__init__.py
Outdated
Show resolved
Hide resolved
compass/landice/tests/calving_dt_convergence/dt_convergence_test/__init__.py
Show resolved
Hide resolved
config_start_time = '0000-01-01_00:00:00' | ||
config_run_duration = '0005-00-00_00:00:00' | ||
config_adaptive_timestep = .true. | ||
config_adaptive_timestep_CFL_fraction = 0.9 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this different from the CFL fraction of 0.95 in https://github.com/MPAS-Dev/compass/pull/386/files#diff-acd02e538bc2c87b44f42baa553a3630dd419f20c83568eac48f8b2a9fc497b7 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(that link is to the namelist.landice file in dt_convergence_test)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One more change I thought of while looking through output figures.
compass/landice/tests/calving_dt_convergence/dt_convergence_test/__init__.py
Outdated
Show resolved
Hide resolved
@trhille , thanks for reviewing this and checking all the output images. I've pushed a commit addressing your review comments. I'm going to work on the docs next, but feel free to look it over now or wait until I have the docs ready. |
21a6ecf
to
8e4909f
Compare
@xylar , this landice PR is finished and just awaiting CI to complete and a final re-run of our integration suite. If you want to review it before Trevor's final re-review and merge, let us know. |
Update: CI and test suites all pass/give expected behavior |
Nope, go ahead. It looks like there are some conflicts. As you know, my preference would be to rebase to handle them but you could either merge |
8e4909f
to
625468c
Compare
This uses the a spunup 2km MISMIP+ domain from Anvil. It runs with data velocity and specified calving velocity for calving. It runs with a series of values of config_adaptive_timestep_calvingCFL_fraction so that the results can be compared. A .png file is created showing the results.
May want to make options to set up either, but for now, just modifying the test, to avoid test proliferation.
Make use of new features added in MALI-Dev/E3SM#33
This is a configuration for investigating convergence of calving behavior with timestep for calving. Rather than duplicating the test in a different test groups, I've instead implemented the test as a test group, and it can be applied to three different meshes: MISMIP+, Humboldt 3km, and Thwaites 4km. It can also be applied to three different calving laws: specified calving velocity, von Mises stress, and eigencalving. It can be applied using either a data velocity field or the FO solver. This implementation replaces the previous commits that applied this same concept to just the MISMIP+ geometry. In each test, a series of runs are conducted that repeat the same simulation using different values of config_adaptive_timestep_calvingCFL_fraction. Then summary plots are generated comparing the behavior for the different fraction values. The calving law parameters are modified for the different configurations as needed to achieve steady retreat at a rate that ensures the calving CFL condition is more restrictive than the advective CFL, even for large values of config_adaptive_timestep_calvingCFL_fraction. The configurations with no velocity solver typically take a few minutes each. The ones with FO velocity take much longer, in some cases a few hours, even after setting those configurations to run for a shorter duration. As coded up now, each simulation with different fractions are run in serial. Setting this up to run them in parallel could be a strategy for getting faster results, if desired. Another limitation is that as coded up now, the analysis/plotting script is run in the validation step and therefore cannot be run manually. Therefore it will only run if all of the individual runs run to completion successfully. This could be improved by moving this operation to a separate step.
The MISMIP+ test group was added in this branch and has only a dt_convergence_test case. Now that I have created a calving_dt_convergence test group, there is no need to have a MISMIP+ test case for that purpose. This commit deletes the dt_convergence_test module and test from the MISMIP+ test group. It then adds a smoke test so that the MISMIP+ test group can actually be used. The smoke test just uses the pre-built initial condition file and runs for 5 years with the FO solver. It takes about a minute and a half on Badger. This test can be modified or replaced with something as needs require.
This brings in the calving CFL PR for MALI that is required to run the new tests: MALI-Dev/E3SM#33
625468c
to
651bd0d
Compare
All tests passed upon re-running the
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@matthewhoffman, great work on this! I've noted a few totally nit-picky things, but am also approving it since these are trivial and do not affect functionality.
This merge adds a new set of tests for MALI that explore convergence with timestep of the calving physics. This is added in a new test group that runs a configuration multiple times with the adaptive timestepper restricted by different fractions of the calculated calving CFL limit. The results are then analyzed and a summary plot is produced. This test can be applied to a number of different calving laws and meshes and using either data velocity or prognostic velocity. These set of tests are not intended to be run routinely as part of integration, but instead are a tool for evaluating development of calving physics. A test suite is also included that runs a number of these tests that have been evaluated as being useful. There are some known ways this test group could be improved, but given its limited intended usage, they are not worth developing further at this time.
A MISMIP+ smoke test is also added as part of this PR. The calving tests were first developed to use the MISMIP+ configuration, which was not yet in COMPASS. Later, things were reorganized to make the calving tests more general and support multiple meshes. Having a MISMIP+ smoke test is generally useful, so that test was retained in this branch after reorganizing things.
Note that this PR includes an update to the MALI-Dev submodule to include the merged MALI PR here: MALI-Dev/E3SM#33.