Skip to content
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

Resolve compset issue #1981

Merged
merged 6 commits into from
Oct 25, 2017
Merged

Conversation

jedwards4b
Copy link
Contributor

A subtle bug fix - the discovery of compsets needs to have information about available components so that it can look in the correct cime_config directories for config_compsets.xml files.

Test suite: scripts_regression_tests.py + cesm F cases
Test baseline:
Test namelist changes:
Test status: bit for bit
Fixes

User interface changes?:

Update gh-pages html (Y/N)?:

Code review:

@@ -433,25 +433,40 @@ def _set_compset(self, compset_name, files):
components = files.get_components("COMPSETS_SPEC_FILE")
logger.debug(" Possible components for COMPSETS_SPEC_FILE are {}".format(components))

cpl_root_dir = files.get_value("COMP_ROOT_DIR_CPL", {"component":"drv"},resolved=False)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, it looks like you have to add the above line and the two lines below every time you want to query a variable in config_files.xml and that variable depends on more than one driver. This is really cumbersome. Is there any way we can wrap this into the XML/files.py class so that a developer would not have to add these lines repeatedly.

@jedwards4b
Copy link
Contributor Author

@mvertens I have rerun testing on this branch. All pass

Copy link
Contributor

@mvertens mvertens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great. Thanks for taking this on. Addressing the minor comments would be really helpful.

# .config_file.xml at the top level may overwrite COMP_ROOT_DIR_ nodes in config_files
if os.path.isfile(config_files_override):
self.read(config_files_override)
for vid in ("COMP_ROOT_DIR_LND", "COMP_ROOT_DIR_ATM", "COMP_ROOT_DIR_ICE",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better not to hardwire the _XXX names such as _LND.

@@ -180,6 +180,8 @@ def _archive_history_files(case, archive, archive_entry,
if not os.path.exists(archive_histdir):
os.makedirs(archive_histdir)
logger.debug("created directory {}".format(archive_histdir))
if compname == 'drv':
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a comment as to why you are doing this.

if len(samenodes) > 1:
expect(len(samenodes) == 2, "Too many matchs for id {} in file {}".format(vid, self.filename))
logger.debug("Overwriting node {}".format(vid))
self.root.remove(samenodes[0])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it always the case that the new node will be after the original node - this is assumed in that your are removing samenodes[0] - the first node. Also - could you please document what this routine is doing in terms of the new functionality we are building in (you are taking the entry from the .config_files in $SRCROOT and overwriting the entry here with that one).

@@ -180,6 +180,7 @@ def _archive_history_files(case, archive, archive_entry,
if not os.path.exists(archive_histdir):
os.makedirs(archive_histdir)
logger.debug("created directory {}".format(archive_histdir))
# the compname is drv but the files are named cpl
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please give an example of what files are named cpl currently so that we can have this documented.

@jedwards4b
Copy link
Contributor Author

@jgfouca this is ready

@jgfouca jgfouca merged commit dd2734a into ESMCI:master Oct 25, 2017
@jedwards4b jedwards4b deleted the resolve_compset_issue branch November 16, 2017 19:56
billsacks pushed a commit to billsacks/cime that referenced this pull request Dec 23, 2017
Resolve compset issue

A subtle bug fix - the discovery of compsets needs to have information about available components so that it can look in the correct cime_config directories for config_compsets.xml files.

Test suite: scripts_regression_tests.py + cesm F cases
Test baseline:
Test namelist changes:
Test status: bit for bit
Fixes

User interface changes?:

Update gh-pages html (Y/N)?:

Code review:
jgfouca pushed a commit that referenced this pull request Apr 26, 2018
The checkpoint timing files, in timing/checkpoints, are named

cesm_timing_<simulation_date_and_time>.

Replace the cesm_timing prefix with model_timing prefix.

Also eliminate the (unnecessary) recursive timer call to CPL:RUN
when do_budgets is .TRUE.

Fixes #2264
Fixes #1981

[BFB]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants