A polynya test case #797
Open
A polynya test case #797
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
Initial commit of a python script to create the setup and a config file to run the test. TODO: - Change the wind direction - Test BBM - Hourly output doesn't work
Most importantly, the wind is blowing at an angle now.
# Conflicts: # run/CMakeLists.txt
Adapts the dirichletFromEdge function to be neumannFromEdge and adds two neumannZero functions - one for DGVectors and one for CGVectors. Both are inspired by the existing dirichletZero function. The neumannZero function must be called after advection - which is a bit worse, but moving it into the DGTransport class is compicated.
This reverts commit b99de51.
Dirichlet: all done using the cglandmask field Neumann: can only appear on the outer edges of the mesh, if the last elements are ice elements.
- fixed advection and mesh test cases
# Conflicts: # dynamics/src/CGDynamicsKernel.cpp # dynamics/src/include/BrittleCGDynamicsKernel.hpp # dynamics/src/include/VPCGDynamicsKernel.hpp
It was there for debugging purposes only.
This makes comparison with the published works easier, as they use something similar to ConfiguredAtmosphere, not FluxConfiguredAtmosphere.
And don't warn about zeros any more. The default rheology is still mEVP (as this is directly comparable to the published work).
The open water heat flux is reset by the thermodynamics, that the (slab) ocean doesn't super cool. This commit sets it back to the initial value on every FluxConfiguredAtmosphere::update call.
Run for the full five days (as previous publications did), use Winton thermodynamics (again) and tweak the ice-atmosphere and open-water heat fluxes.
Added a lot of const-s, changed one equation to be more readable (for me at least), modified the limits to include fixing negative concentration and thickness values, and removed redundant qualifiers.
The test failed, so I ran clang-format on the files.
This is the same now as in Bjornsson et al.
ConfiguredAtmosphere doesn't accept wind_speed as an option anymore, only wind_u and wind_v, so I set them to 3 and 4 to get a wind speed of 5. Also removed an unused header file.
It's not needed any more! Also some clang-format changes.
einola
changed the title
But it still crashes with unstabilities on the boundary after a few time steps.
I reduce the resolution, increase the time step, and run it for five days. This results in a steady state. I then simply compare the max, min, and mean of hice, cice, u, and v with "known good values".
The setup is different over there.
Copy/paste error and python vs python3
This is much more sensitive than I expected. So, I'm only using the DG0 component and not checking the velocities. Running it on macOS vs Linux gives fairly different maximum values for velocities and higher order DG fields.
But it's still useful
|
Now contains only directly polynya-related changes (i.e. config and init files, and an integrtion test) |
einola
changed the title
jwallwork23
requested changes
Jul 17, 2025
There was a problem hiding this comment.
Thanks, this mostly looks good but I have some minor suggestions and a slightly bigger one to avoid duplicated code.
Note that the changes I'm making in #889 will rework the approach taken for scripts in the run directory, but I'm happy to account for this new addition later in my changes.
test/PolynyaIntegration_test.py
Outdated
Comment on lines
108
to
137
| # The model expects everything in metres | ||
| initializer = initMaker(fname, nfirst, nsecond, res*1e3, isWinton=True, checkZeros=False) | ||
|
|
||
| # Ice everywhere and all boundaries closed, except the x = 100 km end | ||
| initializer.mask[:, :] = 1. | ||
| initializer.mask[0, :] = 0. | ||
| initializer.mask[-1, :] = 0. | ||
| initializer.mask[:, 0] = 0. | ||
| #initializer.mask[:, -1] = 0. ## right | ||
|
|
||
| # Uniform concentration of 90% | ||
| initializer.cice[:, :] = 0.9 | ||
|
|
||
| # Uniform thickness of 20 cm | ||
| initializer.hice[:, :] = 0.2 | ||
|
|
||
| # Undamaged ice | ||
| initializer.damage[:, :] = 1. | ||
|
|
||
| # Ice and ocean temperature and salinity at the freezing point | ||
| ice_salinity = 5 # should match Ice::s in constants.hpp | ||
| mu: float = -0.055 # should match Water::mu in constants.hpp | ||
| ocean_temperature = -1.54 | ||
| ocean_salinity = ocean_temperature / mu | ||
|
|
||
| initializer.sss[:, :] = ocean_salinity | ||
| initializer.sst[:, :] = ocean_temperature | ||
| initializer.tsurf[:, :] = ice_salinity * mu | ||
| initializer.tbott = initializer.tsurf | ||
| initializer.tintr = initializer.tsurf |
There was a problem hiding this comment.
This code seems to be duplicated in make_init_polynya.py. It'd be nice to avoid that, if possible.
There was a problem hiding this comment.
Yes, but I'm not sure how to do that sensibly. The test in test/PolynyaIntegration_test.pyshould not be touched, while the script in run/make_init_polynya.py should be an example of how to run one of the cases in the Bjornsson et al paper. So, I don't see how one could reuse code from the other.
There was a problem hiding this comment.
That's true, I suppose it'd need to be a more significant refactor than would be worth doing here.
github-project-automation
bot
moved this from Todo
to Review required
in neXtSIM_DG overview
As suggested by Joe on github. Co-authored-by: Joe Wallwork <22053413+jwallwork23@users.noreply.github.com>
As suggested by Joe on github
That's cleaner than using a fixed string.
As suggested by Joe on github
12 tasks
|
The tests should pass again after #893 is merged into develop. |
einola
added a commit
that referenced
this pull request
Jul 23, 2025
# Correctly skip land points in the stress update Related to #883 ### Task List - [x] Defined the tests that specify a complete and functioning change (*It may help to create a [design specification & test specification](../../../wiki/Specification-Template)*) - [x] Implemented the source code change that satisfies the tests - [x] Documented the feature by providing worked example - [x] Updated the README or other documentation - [x] Completed the pre-Request checklist below --- # Change Description The previous version skipped _ocean_ points and not land points. So I added a not (!) to the commit in PR #884. --- # Test Description The instabilities in #883 are back, but the polynya test case (#797) now runs correctly again. --- # Documentation Impact None --- # Other Details None --- ### Pre-Request Checklist - [x] The requirements of this pull request are fully captured in an issue or design specification and are linked and summarised in the description of this PR - [x] No new warnings are generated - [x] The documentation has been updated (or an issue has been created to track the corresponding change) - [x] Methods and Tests are commented such that they can be understood without having to obtain additional context - [x] This PR/Issue is labelled as a bug/feature/enhancement/breaking change - [x] File dates have been updated to reflect modification date - [x] This change conforms to the conventions described in the README
jwallwork23
approved these changes
Jul 28, 2025
There was a problem hiding this comment.
Thanks for the updates to this @einola, I'm happy with it now.
This test seems almost too sensitive to small changes in the model. Changing the advection scheme caused minor changes in the results; they aren't physically important, but the mean thickness and concentration changed a bit. I don't think this invalidates the test. The mean concentration also changes by 0.001 between platforms.
|
@timspainNERSC: Can you have a look at this one? I can't merge without your approval, because you asked for changes a while ago. |
timspainNERSC
approved these changes
Aug 4, 2025
|
|
||
| nfirst = y // res | ||
| nsecond = x // res | ||
| nLayers = 3 |
There was a problem hiding this comment.
Don't need this any more!
Several changes in develop require coding changes in the script. In particular, there are no netCDF groups in the outputs any more, and temperature is not explicitly initialised when using initMaker.
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.
A polynya test case
Fixes #375
Task List
Change Description
This PR provides a polynya test case (as per issue #375). For this to work, we also had to update the handling of open boundaries. The ice now flows freely out of any open boundaries. Regarding code changes, the most important changes are to the boundary handling, while the polynya test-case setup is valuable as a test for the new boundaries. This PR also includes minor fixes and upgrades to the thermodynamics.
Test Description
The configuration in
config_polynya.cfgreturns results visually similar to those already published (see issue #375).Documentation Impact
None.
Other Details
None
Pre-Request Checklist