Fix resolving reference variables in mixed subgroups

CIME/XML/entry_id.py

@@ -313,7 +313,7 @@ def set_value(self, vid, value, subgroup=None, ignore_type=False):
         )
         node = self.get_optional_child("entry", {"id": vid}, root=root)
         if node is not None:
-            val = self._set_value(node, value, vid, subgroup, ignore_type)
+            val = self._set_value(node, value, vid, None, ignore_type)
         return val
 
     def get_values(self, vid, attribute=None, resolved=True, subgroup=None):
@@ -394,7 +394,7 @@ def _get_value(self, node, attribute=None, resolved=True, subgroup=None):
             val = self.get_default_value(node)
 
         if resolved:
-            val = self.get_resolved_value(val)
+            val = self.get_resolved_value(val, subgroup=subgroup)
 
         return val
 

CIME/XML/env_batch.py

@@ -100,9 +100,9 @@ def get_value(self, item, attribute=None, resolved=True, subgroup=None):
 
     def get_type_info(self, vid):
         gnodes = self.get_children("group")
+        type_info = None
         for gnode in gnodes:
             nodes = self.get_children("entry", {"id": vid}, root=gnode)
-            type_info = None
             for node in nodes:
                 new_type_info = self._get_type_info(node)
                 if type_info is None:

CIME/XML/env_workflow.py

@@ -77,7 +77,6 @@ def get_type_info(self, vid):
         type_info = None
         for gnode in gnodes:
             nodes = self.get_children("entry", {"id": vid}, root=gnode)
-            type_info = None
             for node in nodes:
                 new_type_info = self._get_type_info(node)
                 if type_info is None:

CIME/XML/generic_xml.py

@@ -650,7 +650,7 @@ def get_resolved_value(
         True
         """
         logger.debug("raw_value {}".format(raw_value))
-        reference_re = re.compile(r"\${?(\w+)}?")
+        reference_re = re.compile(r"\${?(?:(.*)::)?(\w+)}?")
         env_ref_re = re.compile(r"\$ENV\{(\w+)\}")
         shell_ref_re = re.compile(r"\$SHELL\{([^}]+)\}")
         math_re = re.compile(r"\s[+-/*]\s")
@@ -677,12 +677,17 @@ def get_resolved_value(
             item_data = item_data.replace(s.group(), run_cmd_no_fail(shell_cmd))
 
         for m in reference_re.finditer(item_data):
-            var = m.groups()[0]
-            logger.debug("find: {}".format(var))
+            _subgroup, var = m.groups()
+
+            logger.debug("find: {} in group {}".format(var, _subgroup))
+
+            if _subgroup is None:
+                _subgroup = subgroup
+
             # The overridden versions of this method do not simply return None
             # so the pylint should not be flagging this
             # pylint: disable=assignment-from-none
-            ref = self.get_value(var, subgroup=subgroup)
+            ref = self.get_value(var, subgroup=_subgroup)
 
             if ref is not None:
                 logger.debug("resolve: " + str(ref))

CIME/case/case.py

@@ -461,19 +461,24 @@ def get_values(self, item, attribute=None, resolved=True, subgroup=None):
             )
             if len(results) > 0:
                 new_results = []
+
                 if resolved:
                     for result in results:
                         if isinstance(result, str):
-                            result = self.get_resolved_value(result)
-                            vtype = env_file.get_type_info(item)
-                            if vtype is not None or vtype != "char":
-                                result = convert_to_type(result, vtype, item)
+                            result = self.get_resolved_value(result, subgroup=subgroup)
+
+                            # If still not resolved, we have a problem
+                            expect(
+                                "$" not in result,
+                                "Could not resolve variable {}".format(item),
+                            )
 
-                            new_results.append(result)
+                            vtype = env_file.get_type_info(item)
 
-                        else:
-                            new_results.append(result)
+                            if vtype is not None and vtype != "char":
+                                result = convert_to_type(result, vtype, item)
 
+                        new_results.append(result)
                 else:
                     new_results = results
 
@@ -483,6 +488,7 @@ def get_values(self, item, attribute=None, resolved=True, subgroup=None):
         return []
 
     def get_value(self, item, attribute=None, resolved=True, subgroup=None):
+        # TODO this needs to be moved into either create_test or create_newcase
         if item == "GPU_ENABLED":
             if not self.gpu_enabled:
                 if (
@@ -492,7 +498,6 @@ def get_value(self, item, attribute=None, resolved=True, subgroup=None):
                     self.gpu_enabled = True
             return "true" if self.gpu_enabled else "false"
 
-        result = None
         for env_file in self._files:
             # Wait and resolve in self rather than in env_file
             result = env_file.get_value(
@@ -502,14 +507,24 @@ def get_value(self, item, attribute=None, resolved=True, subgroup=None):
             if result is not None:
                 if resolved and isinstance(result, str):
                     result = self.get_resolved_value(result, subgroup=subgroup)
+
+                    if "$" in result:
+                        # last ditch effort to get variable from any group
+                        result = self.get_resolved_value(result)
+
+                    # If still not resolved, we have a problem
+                    expect(
+                        "$" not in result, "Could not resolve variable {}".format(item)
+                    )
+
                     vtype = env_file.get_type_info(item)
+
                     if vtype is not None and vtype != "char":
                         result = convert_to_type(result, vtype, item)
 
                 return result
 
-        # Return empty result
-        return result
+        return None
 
     def get_record_fields(self, variable, field):
         """get_record_fields gets individual requested field from an entry_id file
@@ -613,6 +628,12 @@ def set_value(
             "Case must be opened with read_only=False and can only be modified within a context manager",
         )
 
+        if isinstance(value, str):
+            expect(
+                len(value.split("::")) <= 2,
+                f"Value {value!r} is not valid, a namespaced reference must be in the form $SUBGROUP::VARIABLE",
+            )
+
         if item == "CASEROOT":
             self._caseroot = value
         result = None

CIME/tests/test_unit_case.py

@@ -7,7 +7,8 @@
 
 from CIME.case import case_submit
 from CIME.case import Case
-from CIME import utils as cime_utils
+from CIME import utils
+from CIME.tests.utils import mock_case
 
 
 def make_valid_case(path):
@@ -84,9 +85,196 @@ def test_submit(
 
 class TestCase(unittest.TestCase):
     def setUp(self):
-        self.srcroot = os.path.abspath(cime_utils.get_src_root())
+        self.srcroot = os.path.abspath(utils.get_src_root())
         self.tempdir = tempfile.TemporaryDirectory()
 
+    @mock_case()
+    def test_set_value_namespaced_reference(self, case, test_env, **kwargs):
+        test_env.new_entry("HIST_N", "2")
+
+        case.set_value("HIST_N", "$STOP_N")
+
+        assert case.get_value("HIST_N", resolved=False) == "$STOP_N"
+
+        case.set_value("HIST_N", "$test1::STOP_N")
+
+        assert case.get_value("HIST_N", resolved=False) == "$test1::STOP_N"
+
+        with self.assertRaisesRegex(
+            utils.CIMEError,
+            r"ERROR: Value '\$test1::test2::STOP_N' is not valid, a namespaced reference must be in the form \$SUBGROUP::VARIABLE",
+        ):
+            case.set_value("HIST_N", "$test1::test2::STOP_N")
+
+    @mock_case()
+    def test_set_value(self, case, test_env, **kwargs):
+        test_env.new_entry("HIST_N", "2")
+
+        case.set_value("HIST_N", "5")
+
+        hist_n = case.get_value("HIST_N")
+
+        assert hist_n == 5, hist_n
+
+        case.set_value("HIST_N", 10)
+
+        hist_n = case.get_value("HIST_N")
+
+        assert hist_n == 10, hist_n
+
+    @mock_case()
+    def test_get_values_group_reference(self, case, test_env, **kwargs):
+        test_env.new_group("test1")
+        test_env.new_entry("HIST_N", "2, $test2::STOP_N", subgroup="test1")
+
+        test_env.new_group("test2")
+        test_env.new_entry("STOP_N", "4", subgroup="test2")
+
+        outputs = case.get_values("HIST_N")
+
+        assert outputs == [2, 4]
+
+    @mock_case()
+    def test_get_values_reference(self, case, test_env, **kwargs):
+        test_env.new_group("test1")
+        test_env.new_entry("HIST_N", "2, $STOP_N", subgroup="test1")
+
+        test_env.new_entry("STOP_N", "4", subgroup="test1")
+
+        outputs = case.get_values("HIST_N")
+
+        assert outputs == [2, 4]
+
+    @mock_case()
+    def test_get_values(self, case, test_env, **kwargs):
+        test_env.new_group("test1")
+        test_env.new_entry("HIST_N", "2, 4, 6, 8", subgroup="test1")
+
+        outputs = case.get_values("HIST_N")
+
+        assert outputs == [2, 4, 6, 8]
+
+    @mock_case()
+    def test_get_value_group_reference_invalid(self, case, test_env, **kwargs):
+        test_env.new_entry("HIST_N", "$test1::test2::STOP_N")
+
+        with self.assertRaisesRegex(
+            utils.CIMEError,
+            r"ERROR: Could not resolve variable HIST_N",
+        ):
+            case.get_value("HIST_N")
+
+    @mock_case()
+    def test_get_value_nested_group_reference(self, case, test_env, **kwargs):
+        test_env.new_group("test1")
+        test_env.new_group("test2")
+        test_env.new_group("test3")
+
+        test_env.new_entry(
+            "BATCH_COMMAND_FLAGS", "-q $JOB_QUEUE", subgroup="test1", etype="char"
+        )
+        test_env.new_entry(
+            "BATCH_COMMAND_FLAGS",
+            "$test3::BATCH_COMMAND_FLAGS -d -t",
+            subgroup="test2",
+            etype="char",
+        )
+        test_env.new_entry(
+            "BATCH_COMMAND_FLAGS", "-q $JOB_QUEUE", subgroup="test3", etype="char"
+        )
+
+        test_env.new_entry("JOB_QUEUE", "failover", subgroup="test1", etype="char")
+        test_env.new_entry("JOB_QUEUE", "priority", subgroup="test2", etype="char")
+        test_env.new_entry("JOB_QUEUE", "limited", subgroup="test3", etype="char")
+
+        assert case.get_value("BATCH_COMMAND_FLAGS", subgroup="test1") == "-q failover"
+        assert (
+            case.get_value("BATCH_COMMAND_FLAGS", subgroup="test2")
+            == "-q limited -d -t"
+        )
+        assert case.get_value("BATCH_COMMAND_FLAGS", subgroup="test3") == "-q limited"
+
+        case.set_value("BATCH_COMMAND_FLAGS", "-q $test2::JOB_QUEUE", subgroup="test3")
+
+        assert (
+            case.get_value("BATCH_COMMAND_FLAGS", subgroup="test2")
+            == "-q priority -d -t"
+        )
+
+    @mock_case()
+    def test_get_value_group_reference(self, case, test_env, **kwargs):
+        test_env.new_group("test1")
+        test_env.new_entry("HIST_N", "$test3::STOP_N", subgroup="test1")
+        test_env.new_entry("STOP_N", "2", subgroup="test1")
+
+        test_env.new_group("test2")
+        test_env.new_entry("STOP_N", "-2", subgroup="test2")
+
+        test_env.new_group("test3")
+        test_env.new_entry("STOP_N", "5", subgroup="test3")
+
+        hist_n = case.get_value("HIST_N")
+
+        assert hist_n == 5, hist_n
+
+    @mock_case()
+    def test_get_value_reference(self, case, test_env, **kwargs):
+        test_env.new_group("test1")
+        test_env.new_entry("HIST_N", "$STOP_N", subgroup="test1")
+
+        # request no resolve
+        hist_n = case.get_value("HIST_N", resolved=False)
+
+        assert hist_n == "$STOP_N"
+
+        # referenced variable exists nowhere
+        with self.assertRaisesRegex(
+            utils.CIMEError, "ERROR: Could not resolve variable HIST_N"
+        ):
+            case.get_value("HIST_N")
+
+        # referenced variable exists in different subgroup
+        test_env.new_group("test2")
+        test_env.new_entry("STOP_N", "5", subgroup="test2")
+
+        hist_n = case.get_value("HIST_N")
+
+        assert hist_n == 5, hist_n
+
+        test_env.remove_group("test2")
+
+        # referenced variable exists in same subgroup
+        test_env.new_entry("STOP_N", "5", subgroup="test1")
+
+        hist_n = case.get_value("HIST_N")
+
+        assert hist_n == 5, hist_n
+
+        test_env.remove_entry(name="STOP_N", subgroup="test1")
+
+        # referenced variable exists in same subgroup with different type
+        # will convert real to integer, since HIST_N is of integer type
+        test_env.new_entry("STOP_N", "5", subgroup="test1", etype="real")
+
+        hist_n = case.get_value("HIST_N")
+
+        assert hist_n == 5, hist_n
+        assert isinstance(hist_n, int)
+
+    @mock_case()
+    def test_get_value(self, case, test_env, **kwargs):
+        test_env.new_entry("HIST_N", "2")
+
+        hist_n = case.get_value("HIST_N")
+
+        assert hist_n == 2, hist_n
+
+    @mock_case(empty_env=True)
+    def test_get_value_no_files(self, case, **kwargs):
+        hist_n = case.get_value("HIST_N")
+
+        assert hist_n == None, hist_n
+
     @mock.patch("CIME.case.case.Case.read_xml")
     def test_fix_sys_argv_quotes(self, read_xml):
         input_data = ["./xmlquery", "--val", "PIO"]