new error trap if invalid "idmap" file is present in seq_maps.rc #1893

mvertens · 2017-09-15T18:04:45Z

New error trap if invalid "idmap" file is present in the driver input file seq_maps.rc

This PR will result in an error if a mapping file that should exist is instead set to "idmap".
The driver buildnml has been modularized to add this new check more clearly.
There are two hard-wired assumptions that need to be raised as issues.

This check is not done for the fields rof2ocn_fmapname or glc2ice_rmapname. (The rof2ocn_fmapname is used if the run time namelist variable flood_present is set to True).
This check is not done for rof2ocn_* if the ocn component is data ocean. (In this case, this would be triggered by a flood_present flag that should never be on for a data_ocean and is a run time namelist setting).

Test suite: scripts_regression_tests
also ran the prealpha tests on cheyenne with only the namelist compare and verified that
the changes did not change the namelists
Test baseline: cesm2_0_alpha07c
Test namelist changes: Yes
Test status: bit for bit, roundoff

Fixes #962
Fixes #1158
Fixes #1603
User interface changes?: None
Update gh-pages html (Y/N)?: N
Code review: jedwards4b

gold2718

A couple of minor change requests, otherwise okay.

gold2718 · 2017-09-16T23:37:18Z

src/drivers/mct/cime_config/buildnml

@@ -255,7 +303,7 @@ def _create_component_modelio_namelists(case, files):
    if case.get_value("MULTI_DRIVER"):
        maxinst = case.get_value("NINST_MAX")

-    for model in models:
+    for model in case.get_values("COMP_CLASSES"):


Why not just use the models variable?
Or if models is no longer used (does not look used to me), delete it.

Actually, models is not needed as a separate variable any more.

gold2718 · 2017-09-17T01:08:24Z

src/drivers/mct/cime_config/buildnml

+                component1 = name[0:3]
+                component2 = name[4:7]
+                if not ignore_component[component1]  and not ignore_component[component2]:
+                    if name == "rof2ocn_fmapname" or name == "glc2ice_rmapname":


I think this should be a list of names which are currently ignored.

I have done this and will be pushing this back soon - but I think we still will need to have the following explicit - since it is not a mapping file name - and it depends on if the model is "docn" - not sure how else to do it.
if "rof2ocn_" in name:
if case.get_value("COMP_OCN") == 'docn':
logger.warning(" NOTE: ignoring setting of {}=idmap in seq_maps.rc".format(name))

gold2718 · 2017-09-17T05:06:01Z

src/drivers/mct/cime_config/buildnml

+                    if name == "rof2ocn_fmapname" or name == "glc2ice_rmapname":
+                        # TODO: for rof2ocn_fmapname -needs to be resolved since this is
+                        # currently used in prep_ocn_mod.F90 if flood_present is True
+                        logger.warning("   NOTE: ignoring setting of {}=idmap in seq_maps.rc".format(name))


An issue should be opened for this issue.

Ideally, in my mind, we'd have some data-driven list of mapping pairs that are optional (i.e., needed in some configuration but not in others). This can be deferred, though - just mentioning it for the new issue that should be opened.

I have opened an issue on this.

billsacks

A few minor suggestions, none too important (so I'm okay with your leaving things as is if you disagree). Otherwise looks great - thanks for doing this!

billsacks · 2017-09-18T19:20:57Z

src/drivers/mct/cime_config/buildnml

+                    if name == "rof2ocn_fmapname" or name == "glc2ice_rmapname":
+                        # TODO: for rof2ocn_fmapname -needs to be resolved since this is
+                        # currently used in prep_ocn_mod.F90 if flood_present is True
+                        logger.warning("   NOTE: ignoring setting of {}=idmap in seq_maps.rc".format(name))


Ideally, in my mind, we'd have some data-driven list of mapping pairs that are optional (i.e., needed in some configuration but not in others). This can be deferred, though - just mentioning it for the new issue that should be opened.

billsacks · 2017-09-18T19:22:20Z

src/drivers/mct/cime_config/buildnml

+    gridvalue = {}
+    ignore_component = {}
+    exclude_list = ["CPL","ESP"]
+    for item in case.get_values("COMP_CLASSES"):


Renaming 'item' to something like 'compclass' would make this code more self-describing.

Good point. Thanks.

billsacks · 2017-09-18T19:28:12Z

src/drivers/mct/cime_config/buildnml

+    group_variables = nmlgen.get_group_variables("seq_maps")
+    for name in group_variables:
+        value = group_variables[name]
+        if "type" not in name:


It feels like this would be more intuitive and more robust under possible future additions of namelist variables if it were changed to:

if "mapname" in name:

Good point.

mvertens · 2017-09-20T22:49:17Z

I am re-running the scripts regression tests and will send an update when I am read for this to hopefully be merged.

billsacks · 2017-09-21T02:07:19Z

Looks good now, thanks!

mvertens · 2017-09-21T03:14:09Z

Please do not merge yet - I am having some problems with the yellowstone regression tests.

…

On Wed, Sep 20, 2017 at 9:07 PM, goldy ***@***.***> wrote: ***@***.**** approved this pull request. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#1893 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHlxE3Jar5-u8LNdPvpgRmI0mfp2UYGIks5skdLtgaJpZM4PZUdc> .

jedwards4b · 2017-09-21T13:19:42Z

There is a yellowstone problems on the master fixed in PR #1911 - is that the problem you are seeing?

mvertens · 2017-09-21T16:17:03Z

@jedwards4b - I rebased to master and now all tests passed on yellowstone. I have pushed the latest rebase back - and I believe that this PR is now ready to merge.

Enable performance data archiving on Theta Added support for archiving performance data and associated provenance of E3SM jobs on theta. If the user is a member of the OceanClimate group then the data will be saved in performance_archive/ in /projects/OceanClimate . Otherwise the performance data archiving logic is disabled. [BFB]

mvertens assigned jedwards4b Sep 15, 2017

mvertens requested review from gold2718, jedwards4b and billsacks September 15, 2017 18:04

ghost assigned mvertens Sep 15, 2017

ghost added the in progress label Sep 15, 2017

jedwards4b approved these changes Sep 15, 2017

View reviewed changes

jedwards4b added ty: Bug Fix tp: CIMElib labels Sep 15, 2017

gold2718 requested changes Sep 17, 2017

View reviewed changes

billsacks requested changes Sep 18, 2017

View reviewed changes

jedwards4b unassigned mvertens Sep 18, 2017

billsacks approved these changes Sep 21, 2017

View reviewed changes

gold2718 approved these changes Sep 21, 2017

View reviewed changes

mvertens added 4 commits September 20, 2017 21:18

trip idmap values that are invalid before runtime

1aab79d

removed clone directory if create_clone fails

3fbcd74

updates to trapping invalid idmap

594178f

fixed issues raised by reviewers in PR

d7d4838

mvertens force-pushed the mvertens/trip_invalid_idmap branch from a98c61d to d7d4838 Compare September 21, 2017 16:16

jedwards4b merged commit b1d1b9b into master Sep 21, 2017

jedwards4b removed the in progress label Sep 21, 2017

dqwu mentioned this pull request Sep 22, 2017

PR #1893 has caused some cime_developer tests to fail on anlworkstation during phase SETUP #1915

Closed

mvertens pushed a commit that referenced this pull request Sep 22, 2017

fixes #1893

e0d10a2

mvertens mentioned this pull request Sep 22, 2017

fix E3SM idmap check #1917

Merged

mvertens self-assigned this Sep 22, 2017

mvertens deleted the mvertens/trip_invalid_idmap branch October 18, 2017 02:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

new error trap if invalid "idmap" file is present in seq_maps.rc #1893

new error trap if invalid "idmap" file is present in seq_maps.rc #1893

mvertens commented Sep 15, 2017 •

edited by rljacob

Loading

gold2718 left a comment

gold2718 Sep 16, 2017

mvertens Sep 20, 2017

gold2718 Sep 17, 2017

mvertens Sep 20, 2017

gold2718 Sep 17, 2017

billsacks Sep 18, 2017

mvertens Sep 20, 2017

billsacks left a comment

billsacks Sep 18, 2017

billsacks Sep 18, 2017

mvertens Sep 20, 2017

billsacks Sep 18, 2017

mvertens Sep 20, 2017

mvertens commented Sep 20, 2017

billsacks commented Sep 21, 2017

mvertens commented Sep 21, 2017 via email

jedwards4b commented Sep 21, 2017

mvertens commented Sep 21, 2017

new error trap if invalid "idmap" file is present in seq_maps.rc #1893

new error trap if invalid "idmap" file is present in seq_maps.rc #1893

Conversation

mvertens commented Sep 15, 2017 • edited by rljacob Loading

gold2718 left a 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

billsacks left a 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

mvertens commented Sep 20, 2017

billsacks commented Sep 21, 2017

mvertens commented Sep 21, 2017 via email

jedwards4b commented Sep 21, 2017

mvertens commented Sep 21, 2017

mvertens commented Sep 15, 2017 •

edited by rljacob

Loading