variants: improve handling of booleans (#440)

Simplify declaration of boolean variants (see cuda/openmp/rocm
experiment abstract classes for examples).

Boolean variants (e.g. +/~openmp) no longer have to be explicitly
specified (they now correctly use their default value when not
set by the user).

---------

Co-authored-by: pearce8 <[email protected]>

.github/workflows/run.yml

@@ -112,7 +112,7 @@ jobs:
 
       - name: Dry run dynamic amg2023-openmp on Dane with allocation modifier
         run: |
-          ./bin/benchpark experiment init --dest=amg2023-openmp amg2023 openmp=oui
+          ./bin/benchpark experiment init --dest=amg2023-openmp amg2023+openmp
           ./bin/benchpark setup ./amg2023-openmp LLNL-Dane-DELL-sapphirerapids-OmniPath workspace/
           . workspace/setup.sh
           ramble \
@@ -143,7 +143,7 @@ jobs:
 
       - name: Dry run dynamic kripke-openmp on nosite-x86_64 with allocation modifier
         run: |
-          ./bin/benchpark experiment init --dest=kripke-openmp kripke openmp=oui
+          ./bin/benchpark experiment init --dest=kripke-openmp kripke+openmp
           ./bin/benchpark setup ./kripke-openmp nosite-x86_64 workspace/
           . workspace/setup.sh
           ramble \
@@ -154,7 +154,7 @@ jobs:
 
       - name: Dry run dynamic kripke-rocm on LLNL-Tioga-HPECray-zen3-MI250X-Slingshot with allocation modifier
         run: |
-          ./bin/benchpark experiment init --dest=kripke-rocm kripke rocm=oui
+          ./bin/benchpark experiment init --dest=kripke-rocm kripke+rocm
           ./bin/benchpark setup ./kripke-openmp LLNL-Tioga-HPECray-zen3-MI250X-Slingshot workspace/
           . workspace/setup.sh
           ramble \
@@ -186,7 +186,7 @@ jobs:
 
       - name: Dry run dynamic saxpy/rocm with static Tioga
         run: |
-          ./bin/benchpark experiment init --dest=saxpy-rocm saxpy rocm=oui
+          ./bin/benchpark experiment init --dest=saxpy-rocm saxpy+rocm
           ./bin/benchpark setup ./saxpy-rocm LLNL-Tioga-HPECray-zen3-MI250X-Slingshot workspace/
           . workspace/setup.sh
           ramble \
@@ -198,7 +198,7 @@ jobs:
       - name: Dry run dynamic saxpy/rocm with dynamic Tioga
         run: |
           ./bin/benchpark system init --dest=tioga-system2 tioga rocm=551 compiler=cce ~gtl
-          ./bin/benchpark experiment init --dest=saxpy-rocm2 saxpy rocm=oui
+          ./bin/benchpark experiment init --dest=saxpy-rocm2 saxpy+rocm
           ./bin/benchpark setup ./saxpy-rocm2 ./tioga-system2 workspace/
           . workspace/setup.sh
           ramble \
@@ -210,7 +210,7 @@ jobs:
       - name: Dry run dynamic saxpy/cuda with dynamic Sierra
         run: |
           ./bin/benchpark system init --dest=sierra-system sierra cuda=10-1-243 compiler=xl
-          ./bin/benchpark experiment init --dest=saxpy-cuda saxpy cuda=oui
+          ./bin/benchpark experiment init --dest=saxpy-cuda saxpy+cuda
           ./bin/benchpark setup ./saxpy-cuda ./sierra-system workspace/
           . workspace/setup.sh
           ramble \
@@ -262,7 +262,7 @@ jobs:
 
       - name: Dry run dynamic quicksilver-openmp on nosite-x86_64 with allocation modifier
         run: |
-          ./bin/benchpark experiment init --dest=quicksilver-openmp quicksilver openmp=oui experiment=weak
+          ./bin/benchpark experiment init --dest=quicksilver-openmp quicksilver+openmp experiment=weak
           ./bin/benchpark setup ./quicksilver-openmp nosite-x86_64 workspace/
           . workspace/setup.sh
           ramble \
@@ -334,7 +334,7 @@ jobs:
       - name: Dry run dynamic saxpy/openmp with dynamic CTS ruby
         run: |
           ./bin/benchpark system init --dest=ruby-system cts cluster=ruby
-          ./bin/benchpark experiment init --dest=saxpy-openmp saxpy openmp=oui
+          ./bin/benchpark experiment init --dest=saxpy-openmp saxpy+openmp
           ./bin/benchpark setup ./saxpy-openmp ./ruby-system workspace/
           . workspace/setup.sh
           ramble \
@@ -346,7 +346,7 @@ jobs:
       - name: Dry run dynamic saxpy/openmp with dynamic CTS dane
         run: |
           ./bin/benchpark system init --dest=dane-system cts cluster=dane
-          ./bin/benchpark experiment init --dest=saxpy-openmp2 saxpy openmp=oui
+          ./bin/benchpark experiment init --dest=saxpy-openmp2 saxpy+openmp
           ./bin/benchpark setup ./saxpy-openmp2 ./dane-system workspace/
           . workspace/setup.sh
           ramble \
@@ -358,7 +358,7 @@ jobs:
       - name: Dry run dynamic saxpy/openmp with dynamic CTS magma
         run: |
           ./bin/benchpark system init --dest=magma-system cts cluster=magma
-          ./bin/benchpark experiment init --dest=saxpy-openmp3 saxpy openmp=oui
+          ./bin/benchpark experiment init --dest=saxpy-openmp3 saxpy+openmp
           ./bin/benchpark setup ./saxpy-openmp3 ./magma-system workspace/
           . workspace/setup.sh
           ramble \
@@ -370,7 +370,7 @@ jobs:
       - name: Dry run dynamic saxpy/openmp with dynamic generic x86
         run: |
           ./bin/benchpark system init --dest=x86-system genericx86
-          ./bin/benchpark experiment init --dest=saxpy-omp-generic saxpy openmp=oui
+          ./bin/benchpark experiment init --dest=saxpy-omp-generic saxpy+openmp
           ./bin/benchpark setup ./saxpy-omp-generic ./x86-system workspace/
           . workspace/setup.sh
           ramble \

experiments/amg2023/experiment.py

@@ -125,10 +125,10 @@ def compute_applications_section(self):
             for k, v in scaled_variables.items():
                 self.add_experiment_variable(k, v, True)
 
-        if self.spec.satisfies("openmp=oui"):
+        if self.spec.satisfies("+openmp"):
             self.add_experiment_variable("n_ranks", n_resources, True)
             self.add_experiment_variable("n_threads_per_proc", 1, True)
-        elif self.spec.satisfies("cuda=oui") or self.spec.satisfies("rocm=oui"):
+        elif self.spec.satisfies("+cuda") or self.spec.satisfies("+rocm"):
             self.add_experiment_variable("n_gpus", n_resources, True)
 
     def compute_spack_section(self):
@@ -141,16 +141,16 @@ def compute_spack_section(self):
         system_specs["compiler"] = "default-compiler"
         system_specs["mpi"] = "default-mpi"
         system_specs["lapack"] = "lapack"
-        if self.spec.satisfies("cuda=oui"):
+        if self.spec.satisfies("+cuda"):
             system_specs["cuda_version"] = "{default_cuda_version}"
             system_specs["cuda_arch"] = "{cuda_arch}"
             system_specs["blas"] = "cublas-cuda"
-        if self.spec.satisfies("rocm=oui"):
+        if self.spec.satisfies("+rocm"):
             system_specs["rocm_arch"] = "{rocm_arch}"
             system_specs["blas"] = "blas-rocm"
 
         # set package spack specs
-        if self.spec.satisfies("cuda=oui") or self.spec.satisfies("rocm=oui"):
+        if self.spec.satisfies("+cuda") or self.spec.satisfies("+rocm"):
             # empty package_specs value implies external package
             self.add_spack_spec(system_specs["blas"])
         # empty package_specs value implies external package

experiments/kripke/experiment.py

@@ -119,17 +119,17 @@ def compute_applications_section(self):
             for k, v in scaled_variables.items():
                 self.add_experiment_variable(k, v, True)
 
-        if self.spec.satisfies("openmp=oui"):
+        if self.spec.satisfies("+openmp"):
             self.add_experiment_variable("n_ranks", n_resources, True)
             self.add_experiment_variable("n_threads_per_proc", 1, True)
-        elif self.spec.satisfies("cuda=oui") or self.spec.satisfies("rocm=oui"):
+        elif self.spec.satisfies("+cuda") or self.spec.satisfies("+rocm"):
             self.add_experiment_variable("n_gpus", n_resources, True)
 
-        if self.spec.satisfies("openmp=oui"):
+        if self.spec.satisfies("+openmp"):
             self.add_experiment_variable("arch", "OpenMP")
-        elif self.spec.satisfies("cuda=oui"):
+        elif self.spec.satisfies("+cuda"):
             self.add_experiment_variable("arch", "CUDA")
-        elif self.spec.satisfies("rocm=oui"):
+        elif self.spec.satisfies("+rocm"):
             self.add_experiment_variable("arch", "HIP")
 
     def compute_spack_section(self):
@@ -141,10 +141,10 @@ def compute_spack_section(self):
         system_specs = {}
         system_specs["compiler"] = "default-compiler"
         system_specs["mpi"] = "default-mpi"
-        if self.spec.satisfies("cuda=oui"):
+        if self.spec.satisfies("+cuda"):
             system_specs["cuda_version"] = "{default_cuda_version}"
             system_specs["cuda_arch"] = "{cuda_arch}"
-        if self.spec.satisfies("rocm=oui"):
+        if self.spec.satisfies("+rocm"):
             system_specs["rocm_arch"] = "{rocm_arch}"
 
         # set package spack specs

experiments/saxpy/experiment.py

@@ -28,7 +28,7 @@ class Saxpy(Experiment, OpenMPExperiment, CudaExperiment, ROCmExperiment, Calipe
     def compute_applications_section(self):
         # GPU tests include some smaller sizes
         n = ["512", "1024"]
-        if self.spec.satisfies("openmp=oui"):
+        if self.spec.satisfies("+openmp"):
             self.add_experiment_variable("n_nodes", ["1", "2"], True)
             self.add_experiment_variable("n_ranks", "8")
             self.add_experiment_variable("n_threads_per_proc", ["2", "4"], True)

lib/benchpark/cuda.py

@@ -9,12 +9,7 @@
 
 
 class CudaExperiment:
-    variant(
-        "cuda",
-        default="non",
-        values=("oui", "non"),
-        description="Build and run with CUDA",
-    )
+    variant("cuda", default=False, description="Build and run with CUDA")
 
     class Helper(ExperimentHelper):
         def compute_spack_section(self):
@@ -28,7 +23,7 @@ def compute_spack_section(self):
             # set package spack specs
             package_specs = {}
 
-            if self.spec.satisfies("cuda=oui"):
+            if self.spec.satisfies("+cuda"):
                 package_specs["cuda"] = {
                     "pkg_spec": "cuda@{}+allow-unsupported-compilers".format(
                         system_specs["cuda_version"]
@@ -42,11 +37,11 @@ def compute_spack_section(self):
             }
 
         def get_helper_name_prefix(self):
-            return "cuda" if self.spec.satisfies("cuda=oui") else ""
+            return "cuda" if self.spec.satisfies("+cuda") else ""
 
         def get_spack_variants(self):
             return (
                 "+cuda cuda_arch={cuda_arch}"
-                if self.spec.satisfies("cuda=oui")
+                if self.spec.satisfies("+cuda")
                 else "~cuda"
             )

lib/benchpark/openmp.py

@@ -9,16 +9,11 @@
 
 
 class OpenMPExperiment:
-    variant(
-        "openmp",
-        default="non",
-        values=("oui", "non"),
-        description="Build and run with OpenMP",
-    )
+    variant("openmp", default=False, description="Build and run with OpenMP")
 
     class Helper(ExperimentHelper):
         def get_helper_name_prefix(self):
-            return "openmp" if self.spec.satisfies("openmp=oui") else ""
+            return "openmp" if self.spec.satisfies("+openmp") else ""
 
         def get_spack_variants(self):
-            return "+openmp" if self.spec.satisfies("openmp=oui") else "~openmp"
+            return "+openmp" if self.spec.satisfies("+openmp") else "~openmp"

lib/benchpark/rocm.py

@@ -9,20 +9,15 @@
 
 
 class ROCmExperiment:
-    variant(
-        "rocm",
-        default="non",
-        values=("oui", "non"),
-        description="Build and run with ROCm",
-    )
+    variant("rocm", default=False, description="Build and run with ROCm")
 
     class Helper(ExperimentHelper):
         def get_helper_name_prefix(self):
-            return "rocm" if self.spec.satisfies("rocm=oui") else ""
+            return "rocm" if self.spec.satisfies("+rocm") else ""
 
         def get_spack_variants(self):
             return (
                 "+rocm amdgpu_target={rocm_arch}"
-                if self.spec.satisfies("rocm=oui")
+                if self.spec.satisfies("+rocm")
                 else "~rocm"
             )

lib/benchpark/spec.py

@@ -24,17 +24,20 @@
 repo_path = benchpark.repo.paths[benchpark.repo.ObjectTypes.experiments]
 sys_repo = benchpark.repo.paths[benchpark.repo.ObjectTypes.systems]
 
+value_types = str, bool, int, float
+SingleValue = Union[value_types]
+
 
 class VariantMap(llnl.util.lang.HashableMap):
     def __init__(self, init: "VariantMap" = None):
         super().__init__()
         if init:
             self.dict = init.dict.copy()
 
-    def __setitem__(self, name: str, values: Union[str, Iterable]):
+    def __setitem__(self, name: str, values: Union[Iterable, SingleValue]):
         if name in self.dict:
             raise Exception(f"Cannot specify variant {name} twice")
-        if isinstance(values, str):
+        if isinstance(values, value_types):
             values = (values,)
         else:
             values = tuple(values)
@@ -64,10 +67,9 @@ def constrain(self, other: "VariantMap") -> None:
     @staticmethod
     def stringify(name: str, values: tuple) -> str:
         if len(values) == 1:
-            if values[0].lower() == "true":
-                return f"+{name}"
-            if values[0].lower() == "false":
-                return f"~{name}"
+            if isinstance(values[0], bool):
+                prefix = "+" if values[0] else "~"
+                return prefix + name
         v_string = quote_if_needed(",".join(values))
         return f"{name}={v_string}"
 
@@ -84,7 +86,7 @@ def __str__(self):
         bool_keys = []
         kv_keys = []
         for key in sorted_keys:
-            is_bool = len(self[key]) == 1 and self[key][0].lower() in ("true", "false")
+            is_bool = len(self[key]) == 1 and isinstance(self[key][0], bool)
             bool_keys.append(key) if is_bool else kv_keys.append(key)
 
         # add spaces before and after key/value variants.
@@ -648,7 +650,7 @@ def next_spec(self) -> Optional[Spec]:
             if self.ctx.accept(TokenType.BOOL_VARIANT):
                 name = self.ctx.current_token.value[1:].strip()
                 value = self.ctx.current_token.value[0] == "+"
-                spec.variants[name] = str(value).lower()
+                spec.variants[name] = value
             elif self.ctx.accept(TokenType.KEY_VALUE_PAIR):
                 match = SPLIT_KVP.match(self.ctx.current_token.value)
                 assert (

systems/tioga/system.py

@@ -27,7 +27,7 @@ class Tioga(System):
     variant(
         "gtl",
         default=False,
-        values=("true", "false"),
+        values=(True, False),
         description="Use GTL-enabled MPI",
     )