Merge pull request #1323 from douglasjacobsen/fix-variant-issues

Fix issues with expanding variant values

Author: douglasjacobsen

lib/ramble/ramble/cmd/workspace.py

@@ -887,8 +887,13 @@ def workspace_info(args):
 
                     if args.variants:
                         color.cprint(rucolor.nested_4("        Variants: "))
-                        for key, value in app_inst.variants.items():
-                            color.cprint(f"          {key}: {value}")
+                        variant_set = set()
+                        for _, obj in app_inst._objects():
+                            variant_set = variant_set.union(
+                                obj.experiment_variants().as_set(expander=app_inst.expander)
+                            )
+                        for variant in variant_set:
+                            color.cprint(f"          - {variant}")
 
                     if args.executables:
                         color.cprint(rucolor.nested_4("        Executables: "))

lib/ramble/ramble/expander.py

@@ -756,10 +756,12 @@ def satisfies(
                      the input requirement.
         """
 
-        if variant_set is not None:
-            variant_definitions = variant_set.as_set()
-        else:
-            variant_definitions = set()
+        variant_definitions = set()
+
+        if hasattr(variant_set, "as_set"):
+            for variant in variant_set.as_set():
+                exp_variant = self.expand_var(variant)
+                variant_definitions.add(exp_variant)
 
         satisfied = True
         if reqs is not None:

lib/ramble/ramble/test/cmd/workspace.py

@@ -530,7 +530,7 @@ def test_workspace_info_complete(workspace_name):
         assert f"Phases for {pipeline}" in output
 
     assert "Variants:" in output
-    assert "package_manager: spack" in output
+    assert "package_manager=spack" in output
 
     assert "Variables from Workspace" in output
     assert "Variables from Experiment" in output

lib/ramble/ramble/test/variant.py

@@ -288,3 +288,34 @@ def test_modifier_variants_works_with_when(
         with open(ws.config_file_path) as f:
             data = f.read()
             assert expected_spec in data
+
+
+def test_variant_info_works(request):
+    ws_name = request.node.name
+
+    global_args = ["-w", ws_name]
+
+    with ramble.workspace.create(ws_name) as ws:
+        workspace(
+            "manage",
+            "experiments",
+            "when-variants",
+            "--wf",
+            "test_wl",
+            "-v",
+            "n_ranks=1",
+            "-v",
+            "n_nodes=1",
+            "-v",
+            "processes_per_node=1",
+            "-p",
+            "spack",
+            global_args=global_args,
+        )
+
+        ws._re_read()
+        workspace("concretize", global_args=global_args)
+        info_out = workspace("info", "--variants", global_args=global_args)
+
+        assert "application_name=when-variants" in info_out
+        assert "indirect_variant=test-value" in info_out

lib/ramble/ramble/variants.py

@@ -12,6 +12,7 @@
 
 import ramble.error
 import ramble.util.colors as rucolor
+from ramble.expander import Expander
 
 reserved_variants = {
     "modifier",
@@ -249,17 +250,37 @@ def value(self, name: str):
 
         return None
 
-    def as_set(self):
+    def _expanded_set(self, expander: Optional[Expander] = None) -> set:
+        """Return an expanded version of the cached set in this variant set.
+
+        Args:
+            expander (ramble.expander.Expander): Expander to use for expanding this set
+
+        Returns:
+            (set): Set of exanded variant definitions
+        """
+
+        if expander is None:
+            return self._set_cache
+
+        expanded_set = set()
+        for variant in self._set_cache:
+            expanded_set.add(expander.expand_var(variant))
+        return expanded_set
+
+    def as_set(self, expander: Optional[Expander] = None) -> set:
         """Construct a set of definitions for this variant set
 
         The set of variant definitions will be used to determine if a when
         clause is valid or not.
 
         Returns:
             set: A set consisting of strings with the variant definitions
+            expander (ramble.expander.Expander): Expander to use when expanding
+                                                 variant definitions
         """
         if self._set_cache is not None:
-            return self._set_cache
+            return self._expanded_set(expander)
 
         defined_variants = set()
         out_set = set()
@@ -288,7 +309,7 @@ def as_set(self):
                 out_set.add(variant.as_definition())
 
         self._set_cache = out_set
-        return out_set
+        return self._expanded_set(expander)
 
 
 class Variant:
@@ -317,7 +338,7 @@ def copy(self):
             name=self.name, default=self.default, description=self.description, values=self.values
         )
 
-    def as_definition(self):
+    def as_definition(self) -> str:
         """Build a definition for this variant
 
         Format the variant as a string which can be used to test against when
@@ -328,13 +349,6 @@ def as_definition(self):
         """
         return self._definition
 
-        if isinstance(self.default, bool):
-            if self.default:
-                return f"+{self.name}"
-            else:
-                return f"~{self.name}"
-        return f"{self.name}={str(self.default)}"
-
     def as_str(self, n_indent: int = 0, verbose: bool = False):
         """String documentation of this variant
 

var/ramble/repos/builtin.mock/applications/when-variants/application.py

@@ -57,6 +57,19 @@ class WhenVariants(ExecutableApplication):
         description="Variant to control whether validation is on or not",
     )
 
+    variant(
+        "indirect_variant",
+        default="{variant_variable}",
+        description="Variant who's value comes from a variable",
+    )
+
+    workload_variable(
+        "variant_variable",
+        default="test-value",
+        description="Variable to control value of variant",
+        workloads=["*"],
+    )
+
     with when("+validation"):
         register_validator(
             "fixed_n_nodes",

var/ramble/repos/builtin/base_classes/application-base/base_class.py

@@ -598,7 +598,8 @@ def set_variables_and_variants(self, variables, variants, experiment_set):
         for cls in base_chain:
             if hasattr(cls, "name") and cls.name is not None:
                 self.object_variants.multi_value_variant(
-                    "application_name", value=cls.name
+                    "application_name",
+                    value=self.expander.application_name,
                 )
 
         # Define workload_name variant as early as possible