harden eamxx/prod testmod #6621

mahf708 · 2024-09-16T23:47:04Z

makes the ERS eamxx/prod test compare outputs in all files, requiring pulling a bugfix from the scream repo.

github-actions · 2024-09-16T23:50:00Z

PR Preview Action v1.4.7
🚀 Deployed preview to https://E3SM-Project.github.io/E3SM/pr-preview/pr-6621/
on branch `gh-pages` at 2024-09-17 00:25 UTC

rljacob · 2024-09-17T03:15:48Z

cime_config/config_archive.xml

@@ -29,7 +29,7 @@
    <rest_file_extension>r\.(INSTANT|AVERAGE|MAX|MIN)\.n(step|sec|min|hour|day|month|year)s_x\d*</rest_file_extension>
    <rest_file_extension>rhist\.(INSTANT|AVERAGE|MAX|MIN)\.n(step|sec|min|hour|day|month|year)s_x\d*</rest_file_extension>
    <!-- The following matches "hi.AVGTYPE.FREQUNITS_xFREQ.TIMESTAMP.nc"-->


Is this comment still relevant?

No, the only reason I kept is to avoid a merge conflict when the repos are combined: https://github.com/E3SM-Project/scream/blob/25120ff1fd4fb086176d21ebf888d3722a915bb8/cime_config/config_archive.xml#L31-L32.

I also don't think we should keep the logic this way because it is ad-hoc and I had to go to great lengths renaming the prefixes to get it right. We need to settle on a naming convention in the EAMxx IO such that the logic can capture everything without the user having to do special things to make it work. Btw, we already do special thing (disabling the output at t0) to make these comparison tests pass, and we should get rid of that too

mahf708 · 2024-09-17T14:47:50Z

components/eamxx/cime_config/testdefs/testmods_dirs/eamxx/prod/shell_commands

@@ -82,6 +83,8 @@ for file in ${output_yaml_files[@]}; do
        sed -i "s|horiz_remap_file:.*_to_ne30.*|horiz_remap_file: ${hmapfile}|" ./$(basename ${file})
        sed -i "s|horiz_remap_file:.*_to_DecadalSites.*|horiz_remap_file: ${armmapfile}|" ./$(basename ${file})
    fi
+    # replace all filename prefixes so that st_archive works...
+    sed -i "s|eamxx_output.decadal|${case_name}.scream|" ./$(basename ${file})


@rljacob I couldn't get this to work with ".eamxx.[...].h" --- I tried changing all sorts of things (in config_files, in config_component, etc.) to no avail, so I guess we have to keep this way for now?

rljacob · 2024-09-17T15:14:29Z

...onfig/testdefs/testmods_dirs/eamxx/prod/yaml_outs/eamxx_output.decadal.1dailyAVG_native.yaml

@@ -1,6 +1,6 @@
 %YAML 1.1
 ---
-filename_prefix: scream_output.decadal.1dailyAVG_native.h
+filename_prefix: eamxx_output.decadal.1dailyAVG_native.h


The general pattern should be [CASE_name].[model name].h[anystring].[date string].nc
so make this output.decadal.eamxx.h.1dailyAVG_native then in your sed command replace "output.decadal" with "case_name".

I agree.

Then, we have to change this line, right?

<hist_file_extension>.*\.h\.(?!rhist\.).*\.nc$</hist_file_extension>

Currently, this captures: [CASE_name].[model_name].[anystring].h.[date_misc_string].nc

--

Either way, we need to hard-wire the logic (I prefer yours, because it is consistent with other components) in the EAMxx IO. Imho, that should be one of the first steps in reconciling the repos. I can try to do it after this PR.

For this PR, I can add a second hist_file_extension entry to capture your proposed logic

You should be able to use the same regex as eam:
<hist_file_extension>h\d*.*\.nc$</hist_file_extension>

The problem is, EAMxx IO limitation mean that we save the rhist files with the same exact pattern except with "rhist" somewhere before the date info, so we need to exclude those somehow

And rhist files are saved regardless if they're needed or not in EAMxx... at every restart

No, not yet :) it's only in the preset-output tests and this PR. But it is really time we make that a commitment from the EAMxx IO to have a clear signature. I will bring it up during the dev call this week and if people are happy, we can make a permanent change such that we guarantee EAMxx IO saves files with a specific pattern to make everyone happy (tests, Rob, etc.)

Ah, I see that Jim changed the prefix in the testmods. I thought we actually made the change here, making it return .h.

The question to settle is whether not we want to adopt a stricter pattern, that is, which one of these do we want?

[anystring].h.[date_specs].nc

[case_name].[model_name].h.[anystring].[date_specs].nc

Note that we do need the model name somewhere (so we maybe should make the code return scream.h, scream.r, scream.rhist, etc.?)

But this conversation is not a blocker for the PR, so we can discuss it separately

Can you create an issue while we wait for the dev call? I think this can be settled in 1 min of typing by a handful of ppl... I'm ok with either pattern, as long as we allow the presence of [anystring] somewhere (rather than rely on h0, h1, h2, ... numbering of the streams).

Done: E3SM-Project/scream#3001

mahf708 · 2024-09-17T20:39:58Z

@rljacob one thing that was pretty confusing to me: I don't see anything in the logs about 1994 (start date of this run). But I do see them once I download the saved logs. How can we get ./create_test to print information that's similar to what we see on cdash?

bartgol · 2024-09-17T21:15:47Z

cime_config/config_archive.xml

@@ -29,7 +29,7 @@
    <rest_file_extension>r\.(INSTANT|AVERAGE|MAX|MIN)\.n(step|sec|min|hour|day|month|year)s_x\d*</rest_file_extension>
    <rest_file_extension>rhist\.(INSTANT|AVERAGE|MAX|MIN)\.n(step|sec|min|hour|day|month|year)s_x\d*</rest_file_extension>
    <!-- The following matches "hi.AVGTYPE.FREQUNITS_xFREQ.TIMESTAMP.nc"-->
-    <hist_file_extension>hi\.(INSTANT|AVERAGE|MAX|MIN)\.n(step|sec|min|hour|day|month|year)s_x\d*\.\d{4}-\d{2}-\d{2}-\d{5}\.nc$</hist_file_extension>
+    <hist_file_extension>.*\.h\.(?!rhist\.).*\.nc$</hist_file_extension>


What's the point of the negative look ahead? There should be no ".h.rhist" file generated by EAMxx. If someone puts ".h" in the filename prefix, they are kind of asking for it...

I just copied what @jgfouca did: E3SM-Project/scream#2957

rljacob · 2024-09-20T17:23:34Z

Is this ready?

mahf708 · 2024-09-20T17:46:45Z

Yes, it's ready to be merged. We will follow up with another PR to add this testmod to more nightly tests, but for now, we will just run it in here. Thanks!

harden eamxx/prod testmod makes the ERS eamxx/prod test compare outputs in all files, requiring pulling a bugfix from the scream repo. [BFB]

fix outdated path of output yaml files

a56bd4e

mahf708 added Testing Anything related to unit/system tests EAMxx PRs focused on capabilities for EAMxx labels Sep 16, 2024

edit logic to accomodate eamxx specs

6a6415a

mahf708 force-pushed the mahf708/ig/eamxx-prod-testing branch from 430163a to 6a6415a Compare September 17, 2024 01:00

mahf708 requested review from bartgol, rljacob and jgfouca September 17, 2024 01:32

mahf708 marked this pull request as ready for review September 17, 2024 01:39

rljacob reviewed Sep 17, 2024

View reviewed changes

mahf708 commented Sep 17, 2024

View reviewed changes

rljacob reviewed Sep 17, 2024

View reviewed changes

rljacob self-assigned this Sep 17, 2024

bartgol reviewed Sep 17, 2024

View reviewed changes

jgfouca approved these changes Sep 20, 2024

View reviewed changes

rljacob assigned jgfouca and unassigned rljacob Sep 20, 2024

rljacob approved these changes Sep 20, 2024

View reviewed changes

jgfouca added a commit that referenced this pull request Sep 20, 2024

Merge branch 'mahf708/ig/eamxx-prod-testing' into next (PR #6621)

ec9a730

harden eamxx/prod testmod makes the ERS eamxx/prod test compare outputs in all files, requiring pulling a bugfix from the scream repo. [BFB]

jgfouca merged commit efe4269 into master Sep 20, 2024
9 checks passed

jgfouca deleted the mahf708/ig/eamxx-prod-testing branch September 20, 2024 18:43

mahf708 mentioned this pull request Oct 3, 2024

Fix bug related to name clashing in output streams E3SM-Project/scream#2986

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

harden eamxx/prod testmod #6621

harden eamxx/prod testmod #6621

mahf708 commented Sep 16, 2024 •

edited

Loading

github-actions bot commented Sep 16, 2024 •

edited

Loading

rljacob Sep 17, 2024

mahf708 Sep 17, 2024

mahf708 Sep 17, 2024

rljacob Sep 17, 2024

mahf708 Sep 17, 2024

rljacob Sep 17, 2024

mahf708 Sep 17, 2024

mahf708 Sep 17, 2024

mahf708 Sep 18, 2024

mahf708 Sep 18, 2024

mahf708 Sep 18, 2024

bartgol Sep 18, 2024

mahf708 Sep 18, 2024

mahf708 commented Sep 17, 2024

bartgol Sep 17, 2024

mahf708 Sep 17, 2024

rljacob commented Sep 20, 2024

mahf708 commented Sep 20, 2024

harden eamxx/prod testmod #6621

harden eamxx/prod testmod #6621

Conversation

mahf708 commented Sep 16, 2024 • edited Loading

github-actions bot commented Sep 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mahf708 commented Sep 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rljacob commented Sep 20, 2024

mahf708 commented Sep 20, 2024

mahf708 commented Sep 16, 2024 •

edited

Loading

github-actions bot commented Sep 16, 2024 •

edited

Loading