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

Add comment pes file #2315

Merged
merged 5 commits into from
Feb 23, 2018
Merged

Add comment pes file #2315

merged 5 commits into from
Feb 23, 2018

Conversation

jedwards4b
Copy link
Contributor

Add the comment from config_pes.xml to the env_mach_pes.xml file

Test suite: scripts_regression_tests.py
Test baseline:
Test namelist changes:
Test status: bit for bit

Fixes #2248

User interface changes?:

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

Code review:

@@ -17,6 +18,12 @@ def __init__(self, case_root=None, infile="env_mach_pes.xml", components=None):
schema = os.path.join(get_cime_root(), "config", "xml_schemas", "env_mach_pes.xsd")
EnvBase.__init__(self, case_root, infile, schema=schema)

def add_comment(self, comment):
if comment is not None:
tmp = GenericXML(root_name_override="comment", infile="fake", read_only=False)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a bit awkward, but I couldn't find a cleaner way.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jedwards4b , I see now that make_child cannot support what you want since it attaches the child as a SubElement. Probably the easiest thing here would be:

comment = self.make_child("comment", text=comment)
self.remove_child(comment)
self.add_child(comment, position=1)

Copy link
Contributor

@jgfouca jgfouca left a comment

Choose a reason for hiding this comment

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

See inline

@@ -17,6 +18,12 @@ def __init__(self, case_root=None, infile="env_mach_pes.xml", components=None):
schema = os.path.join(get_cime_root(), "config", "xml_schemas", "env_mach_pes.xsd")
EnvBase.__init__(self, case_root, infile, schema=schema)

def add_comment(self, comment):
if comment is not None:
tmp = GenericXML(root_name_override="comment", infile="fake", read_only=False)
Copy link
Contributor

Choose a reason for hiding this comment

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

@jedwards4b , I see now that make_child cannot support what you want since it attaches the child as a SubElement. Probably the easiest thing here would be:

comment = self.make_child("comment", text=comment)
self.remove_child(comment)
self.add_child(comment, position=1)

"""
Add element node to self at root
"""
expect(not self.locked and not self.read_only, "locked")
root = root if root is not None else self.root
root.xml_element.append(node.xml_element)
if position > 0:
Copy link
Contributor

Choose a reason for hiding this comment

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

0 is a valid position. I think we want position=None to be default and this if check should check against None.

@jedwards4b
Copy link
Contributor Author

Not sure that your suggestion is any less awkward than what I came up with?

@jgfouca
Copy link
Contributor

jgfouca commented Feb 23, 2018

@jedwards4b , I think it's a bit better since it doesn't involve creating an entire fake file.

@jedwards4b
Copy link
Contributor Author

done

@jgfouca jgfouca merged commit 93dde2b into ESMCI:master Feb 23, 2018
@jedwards4b jedwards4b deleted the add_comment_pes_file branch February 23, 2018 21:56
jgfouca pushed a commit that referenced this pull request May 18, 2018
Various software modules have been removed/updated as part of a major
update.

This addresses runtime issues with JSM identified by @worleyph.
spectrum-mpi/10.2.0.0-20180406 is the only supported version.

PGI compilers updated to 18.3
XL updated to xl/20180406-beta.
Conditional batch directives used for gmumps, smt2 options. Refer #1906
Discussion about Summit task placement at
https://acme-climate.atlassian.net/wiki/spaces/PERF/pages/645234702/Summit+Task+Placement
Limited testing with FC5AV1C-ne4 and PGI.

Note: XL compilers still have a build issue with mct but this is the
only XL version with compatible MPI module. Ongoing work to identify root
cause.

[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.

2 participants