-
Notifications
You must be signed in to change notification settings - Fork 207
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
Updates MVK test to be configured through testmod #4618
base: master
Are you sure you want to change the base?
Conversation
TODO:
|
You can preview documentation at https://esmci.github.io/cime/branch/update-mvk-config/html/index.html |
New relevant doc section https://esmci.github.io/cime/branch/update-mvk-config/html/users_guide/testing.html#system-test-mods |
@mkstratos please review. |
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.
I was able to run a default MVK test, and a modified MVK test with testmod capability, and it looks great! I just have a few minor comments (mostly due to undocumented changes to EVV).
CIME/SystemTests/mvk.py
Outdated
) | ||
self._set_attribute("ninst", 30, "The number of instances.") | ||
self._set_attribute( | ||
"critical", 13, "The critical value for rejecting the null hypothese." |
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.
EVV no longer needs this as of v0.5.0, (implemented false discovery rate correction), but that's my fault as it's not fully documented! It won't cause an error, but it isn't used by evv's ks.py
CIME/SystemTests/mvk.py
Outdated
evv_lib_dir = os.path.abspath(os.path.dirname(evv4esm.__file__)) | ||
version = evv4esm.__version_info__ | ||
|
||
assert version[0] <= 0 and version[1] <= 5, "Please install evv4esm less than 0.5.x" |
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.
This would need updating each time there's a new EVV version, maybe this should be a lower bound instead, all the changes are compatible with evv4esm >= 0.5.0
CIME/SystemTests/mvk.py
Outdated
set_nml_variable("seed_custom", f"{iinst}") | ||
set_nml_variable("seed_clock", ".true.") | ||
|
||
def test_config( |
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.
Could this instead be called test_config_evv
? That way it's clear that this method is for creating the configuration to be passed to evv4esm?
doc/source/users_guide/testing.rst
Outdated
component str The main component. | ||
components [] list Components that require namelist customization. | ||
ninst 30 int The number of instances. | ||
critical 13 int The critical value for rejecting the null hypothese. |
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.
Same comment as on mvk.py
, just that I didn't document this (yet!).
@mkstratos Thanks for the feedback, it looks great. |
@rljacob @jgfouca @jedwards4b @mkstratos from CIME.utils import safe_copy
from CIME.utils import run_cmd
component = "eamxx"
ninst = 30
def generate_namelist(case, component, i, filename):
run_cmd(f"atmchange initial_conditions::perturbation_random_seed = {i*32}")
run_cmd(f"atmchange ...")
...
safe_copy("namelist_scream.xml", f"namelist_scream_{i:04}.xml") |
Okay with me. |
Fine with me too. Feel free to merge it whenever you think it's ready. |
Updates MVK systemtest to be configure through a testmod. Add a
param.py
file in the testmod directory. Runningpython -c "from CIME.SystemTests.mvk import MVKConfig; MVKConfig().print_rst_table()
will print out documentation for the settings/functions.
Test suite: n/a
Test baseline: n/a
Test namelist changes: n/a
Test status: n/a
Fixes #4592
User interface changes?:
Update gh-pages html (Y/N)?: