fix wrong graupel tuning parameter config in data test#1075
fix wrong graupel tuning parameter config in data test#1075
Conversation
|
cscs-ci run default |
|
cscs-ci run distributed |
| rain_mu=0.0, | ||
| rain_n0=1.0, | ||
| snow2graupel_riming_coeff=0.5, | ||
| ice_stickeff_min=0.075, |
There was a problem hiding this comment.
Don't hardcode values in the middle of the test. Move to definitions.py where all other experiments coefficients are defined. Add a construct_mycrophisics_config method maybe?
There was a problem hiding this comment.
Good to know many config is summarized in one place in definitions.py, and I think this microphysics config may be (a big question mark??) used again in the driver test . But it looks like it is still not very nice at all, it seems to me that we hardcode at another place. Similarly for VerticalGridConfig, this is kind of hardcoded everywhere... Possibly have to wait until we serialize namelist parameters and read them and generate config automatically somewhere.
There was a problem hiding this comment.
they are serialized, I've added all NAMELIST_* files to serialized data, if you want you can start writing a reader for that and take them from there instead of hardcoding new stuff ;-)
There was a problem hiding this comment.
model_top_height is defined in testing/definitions.py::construct_metrics_config with a different value
get the value from there, update it if necessary
There was a problem hiding this comment.
Hmm, and many other parameters are wrong
There was a problem hiding this comment.
as above, maybe it's a good idea if you write a small reader so we start with this PR to get them from the serialized data
|
cscs-ci run default |
|
cscs-ci run distributed |
|
try this: c8f0021 |
…erization options
| ) | ||
| #: snow size distribution interception parameter. Originally defined as isnow_n0temp (PARAMETER) in gscp_data.f90 in ICON. I keep it because I think the choice depends on resolution. | ||
| snow_intercept_option: mphys_options.SnowInterceptParametererization = ( | ||
| snow_intercept_option: mphys_options.ValidSnowInterceptParametererization = ( |
There was a problem hiding this comment.
@havogt is this correct? otherwise, mypy complained about wrong type
|
Mandatory Tests Please make sure you run these tests via comment before you merge!
Optional Tests To run benchmarks you can use:
To run tests and benchmarks with the DaCe backend you can use:
To run test levels ignored by the default test suite (mostly simple datatest for static fields computations) you can use:
For more detailed information please look at CI in the EXCLAIM universe. |
| def read_namelist(path: pathlib.Path) -> dict: | ||
| """ICON NAMELIST_ICON_output_atm reader. | ||
| Returns a dictionary of dictionaries, where the keys are the namelist names | ||
| and the values are dictionaries of key-value pairs from the namelist. | ||
|
|
||
| Use as: | ||
| namelists = read_namelist("/path/to/NAMELIST_ICON_output_atm") | ||
| print(namelists["NWP_TUNING_NML"]["TUNE_ZCEFF_MIN"]) | ||
| """ | ||
| with path.open() as f: | ||
| txt = f.read() | ||
| namelist_set = re.findall(r"&(\w+)(.*?)\/", txt, re.S) | ||
| full_namelist = {} | ||
| for namelist_name, namelist_content in namelist_set: | ||
| full_namelist[namelist_name] = _parse_namelist_content(namelist_content) | ||
| return full_namelist | ||
|
|
||
|
|
||
| def _parse_namelist_content(namelist_content): | ||
| """ | ||
| Parse the contents of a single namelist group to a Python dictionary. | ||
| """ | ||
| result = {} | ||
| current_variable = None | ||
|
|
||
| # Remove comments | ||
| namelist_content = re.sub(r"!.*", "", namelist_content) | ||
|
|
||
| # Split into lines | ||
| lines = namelist_content.splitlines() | ||
|
|
||
| for line in lines: | ||
| line = line.strip() | ||
| # skip any non-meaningful empty line | ||
| if not line: | ||
| continue | ||
|
|
||
| # Remove trailing comma | ||
| if line.endswith(","): | ||
| line = line[:-1] | ||
|
|
||
| if "=" in line: | ||
| # New variable-value pair | ||
| variable, value = line.split("=", 1) | ||
| current_variable = variable.strip() | ||
| result[current_variable] = _parse_value(value) | ||
| # TODO(Chia Rui): check if continuation lines is irrelevant in our tests | ||
| # else: | ||
| # # Continuation line (array or multi-line string) | ||
| # if current_variable is not None: | ||
| # val = _parse_value(line) | ||
| # # convert to a list if we have multiple values for the same variable | ||
| # if not isinstance(result[current_variable], list): | ||
| # result[current_variable] = [result[current_variable]] | ||
| # result[current_variable].append(val) |
There was a problem hiding this comment.
actually there is a library https://f90nml.readthedocs.io/en/latest/
import f90nml
cfg = f90nml.read("NAMELIST_ICON_output_atm")
print(cfg["time_nml"]["calendar"]) # keys are lowercase by default
print("Namelists:", len(nml))
print("Names:", list(nml.keys()))
There was a problem hiding this comment.
😂😂 I also just found that when I asked chatGPT (feeling I wasted time on refactoring😭😂)
There was a problem hiding this comment.
we can save those lines here
There was a problem hiding this comment.
nah, just delete everything and use the library, way more reliable and tested
There was a problem hiding this comment.
I actually mean we can "save" a lot of lines of changes by using the library😉
There was a problem hiding this comment.
ahaaaa yes I agree! :-) good thing we both found out about it!
2 participants
The data test
test_graupelintest_single_moment_six_class_gscp_graupel.pyfails after the serialized data is updated in PR#1004.The reason is that the config parameters for the microphysics scheme are different from the corresponding namelist parameters in the run script. In particular,
tune_icesedi_expandtune_zceff_minare wrong.This PR fixes the wrong config parameters.