Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 34 additions & 13 deletions scripts/lib/CIME/XML/entry_id.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,29 +43,45 @@ def set_default_value(self, vid, val):

return val

def get_value_match(self, vid, attributes=None, exact_match=False, entry_node=None):
# Handle this case:
# <entry id ...>
# <values>
# <value A="a1">X</value>
# <value A="a2">Y</value>
# <value A="a3" B="b1">Z</value>
# </values>
# </entry>
def get_value_match(self, vid, attributes=None, exact_match=False, entry_node=None,
replacement_for_none=None):
"""Handle this case:
<entry id ...>
<values>
<value A="a1">X</value>
<value A="a2">Y</value>
<value A="a3" B="b1">Z</value>
</values>
</entry>

If replacement_for_none is provided, then: if the found text value would give a
None value, instead replace it with the value given by the replacement_for_none
argument. (However, still return None if no match is found.) This may or may not
be needed, but is in place to maintain some old logic.

"""

if entry_node is not None:
value = self._get_value_match(entry_node, attributes, exact_match)
value = self._get_value_match(entry_node, attributes, exact_match,
replacement_for_none=replacement_for_none)
else:
node = self.get_optional_child("entry", {"id":vid})
value = None
if node is not None:
value = self._get_value_match(node, attributes, exact_match)
value = self._get_value_match(node, attributes, exact_match,
replacement_for_none=replacement_for_none)
logger.debug("(get_value_match) vid {} value {}".format(vid, value))
return value

def _get_value_match(self, node, attributes=None, exact_match=False):
def _get_value_match(self, node, attributes=None, exact_match=False,
replacement_for_none=None):
'''
Note that the component class has a specific version of this function

If replacement_for_none is provided, then: if the found text value would give a
None value, instead replace it with the value given by the replacement_for_none
argument. (However, still return None if no match is found.) This may or may not
be needed, but is in place to maintain some old logic.
'''
# if there is a <values> element - check to see if there is a match attribute
# if there is NOT a match attribute, then set the default to "first"
Expand Down Expand Up @@ -125,7 +141,12 @@ def _get_value_match(self, node, attributes=None, exact_match=False):
expect(False,
"match attribute can only have a value of 'last' or 'first', value is %s" %match_type)

return self.text(mnode)
text = self.text(mnode)
if text is None:
# NOTE(wjs, 2021-06-03) I'm not sure when (if ever) this can happen, but I'm
# putting this logic here to maintain some old logic, to be safe.
text = replacement_for_none
return text

def get_node_element_info(self, vid, element_name):
node = self.get_optional_child("entry", {"id":vid})
Expand Down
18 changes: 13 additions & 5 deletions scripts/lib/CIME/XML/namelist_definition.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,10 @@ def get_group(self, name):
def add_attributes(self, attributes):
self._attributes = attributes

def get_attributes(self):
"""Return this object's attributes dictionary"""
return self._attributes

def get_entry_nodes(self):
return self._entry_nodes

Expand All @@ -174,6 +178,9 @@ def set_value(self, vid, value, subgroup=None, ignore_type=True):
"""This function is not implemented."""
raise TypeError("NamelistDefinition does not support `set_value`.")

# In contrast to the entry_id version of this method, this version doesn't support the
# replacement_for_none argument, because it is hard-coded to ''.
# pylint: disable=arguments-differ
def get_value_match(self, vid, attributes=None, exact_match=True, entry_node=None):
"""Return the default value for the variable named `vid`.

Expand All @@ -190,11 +197,12 @@ def get_value_match(self, vid, attributes=None, exact_match=True, entry_node=Non

if entry_node is None:
entry_node = self._nodes[vid]
# NOTE(wjs, 2021-06-04) In the following call, replacement_for_none='' may not
# actually be needed, but I'm setting it to maintain some old logic, to be safe.
value = super(NamelistDefinition, self).get_value_match(vid.lower(),attributes=all_attributes, exact_match=exact_match,
entry_node=entry_node)
if value is None:
value = ''
else:
entry_node=entry_node,
replacement_for_none='')
Comment on lines +200 to +204
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

As I say in the comment, I don't know if this replacement_for_none is actually needed; I added it to be safe to maintain existing logic. @jedwards4b @jgfouca do you know off-hand if a None value can be generated from the text value retrieved by entry_id's get_value_match? If you'd like, I can do some testing of whether this is ever triggered in CESM testing and remove it if it isn't.

Also: I don't love that the interface of get_value_match now differs from the entry_id version (requiring #pylint: disable=arguments-differ), but this seemed better to me than other implementations I could think of. I'm open to suggestions if you see a better way to do this.

if value is not None:
value = self._split_defaults_text(value)

return value
Expand Down Expand Up @@ -445,4 +453,4 @@ def get_default_value(self, item, attribute=None):
all_attributes.update(attribute)

value = self.get_value_match(item.lower(), all_attributes, True)
return self._split_defaults_text(value)
return value
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The original motivation for this change was seeing that self.get_value_match can now return None; but as I looked closely, I saw that there was a double-call to self._split_defaults_text(value) (at the end of self.get_value_match and here) that looks like a bug, so I have removed that double-call. I don't see any calls to this get_default_value routine from within cime, but it's possible that components use it in their buildnml.

4 changes: 3 additions & 1 deletion scripts/lib/CIME/namelist.py
Original file line number Diff line number Diff line change
Expand Up @@ -953,7 +953,9 @@ def get_variable_value(self, group_name, variable_name):
if gn:
vn = string_in_list(variable_name,self._groups[gn])
if vn:
return self._groups[gn][vn]
# Make a copy of the list so that any modifications done by the caller
# don't modify the internal values.
return self._groups[gn][vn][:]
return ['']

def get_value(self, variable_name):
Expand Down
5 changes: 4 additions & 1 deletion scripts/lib/CIME/nmlgen.py
Original file line number Diff line number Diff line change
Expand Up @@ -629,6 +629,8 @@ def add_default(self, name, value=None, ignore_abs_path=None):
have_value = False
# Check for existing value.
current_literals = self._namelist.get_variable_value(group, name)
if current_literals != [""]:
have_value = True

# Check for input argument.
if value is not None:
Expand All @@ -643,7 +645,8 @@ def add_default(self, name, value=None, ignore_abs_path=None):
have_value = True
default_literals = self._to_namelist_literals(name, default)
current_literals = merge_literal_lists(default_literals, current_literals)
expect(have_value, "No default value found for {}.".format(name))
expect(have_value, "No default value found for {} with attributes {}.".format(
name, self._definition.get_attributes()))

# Go through file names and prepend input data root directory for
# absolute pathnames.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@
<value component="glc">$GLC_PIO_NETCDF_FORMAT</value>
<value component="wav">$WAV_PIO_NETCDF_FORMAT</value>
<value component="iac">$IAC_PIO_NETCDF_FORMAT</value>
<value component="esp">$ESP_PIO_NETCDF_FORMAT</value>
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm not sure if this is the right setting: other esp pio values seem to be set to some "undefined" value. But this seemed more right than the earlier behavior, where this was left as blank in the namelist file.

</values>
</entry>

Expand Down