Merge pull request #951 from douglasjacobsen/concretize-permits-errors

Permit errors when concretizing workspace

Author: rfbgo

lib/ramble/ramble/application.py

@@ -903,10 +903,14 @@ def build_modifier_instances(self):
                 self.expander.add_no_expand_var(var)
                 mod_inst.expander.add_no_expand_var(var)
 
-    def validate_experiment(self):
+    def validate_experiment(self, warn_validation=True, die_on_validate_error=True):
         # Validate the new modifiers variables exist
         # (note: the base ramble variables are checked earlier too)
-        self.keywords.check_required_keys(self.variables)
+        self.keywords.check_required_keys(
+            self.variables,
+            warn_validation=warn_validation,
+            die_on_validate_error=die_on_validate_error,
+        )
         self._check_object_validators()
 
     def _check_object_validators(self):

lib/ramble/ramble/experiment_set.py

@@ -138,7 +138,7 @@ def set_workload_context(self, workload_context):
 
         self._set_context(self._contexts.workload, workload_context)
 
-    def set_experiment_context(self, experiment_context):
+    def set_experiment_context(self, experiment_context, die_on_validate_error=True):
         """Set up current experiment context"""
 
         try:
@@ -148,7 +148,7 @@ def set_experiment_context(self, experiment_context):
             raise RambleVariableDefinitionError(f"In experiment {namespace}: {e}")
 
         self._set_context(self._contexts.experiment, experiment_context)
-        self._ingest_experiments()
+        self._ingest_experiments(die_on_validate_error=die_on_validate_error)
 
     @property
     def application_namespace(self):
@@ -253,7 +253,15 @@ def _compute_mpi_vars(self, expander, variables):
         if not n_threads:
             variables[self.keywords.n_threads] = 1
 
-    def _prepare_experiment(self, exp_template_name, variables, context, repeats):
+    def _prepare_experiment(
+        self,
+        exp_template_name,
+        variables,
+        context,
+        repeats,
+        die_on_validate_error=True,
+        warn_validation=True,
+    ):
         """Prepare an experiment instance
 
         Create an experiment instance based on the input variables, context,
@@ -268,6 +276,7 @@ def _prepare_experiment(self, exp_template_name, variables, context, repeats):
         Returns:
             (Application): Instance of an application class for this experiment
         """
+
         experiment_suffix = ""
         # After generating the base experiment, append the index to repeat experiments
         if repeats.repeat_index:
@@ -348,14 +357,9 @@ def _prepare_experiment(self, exp_template_name, variables, context, repeats):
         app_inst.add_expand_vars(self._workspace)
         app_inst.read_status()
 
-        try:
-            app_inst.validate_experiment()
-        except ramble.keywords.RambleKeywordError as e:
-            raise RambleVariableDefinitionError(str(e))
-
         return app_inst
 
-    def _ingest_experiments(self):
+    def _ingest_experiments(self, die_on_validate_error=True):
         """Ingest experiments based on the current context.
 
         Merge all contexts, and render individual experiments. Track these
@@ -463,7 +467,12 @@ def _ingest_experiments(self):
             tracking_group, exclude_where=exclude_where, ignore_used=False, fatal=False
         ):
             app_inst = self._prepare_experiment(
-                experiment_template_name, tracking_vars, final_context, repeats
+                experiment_template_name,
+                tracking_vars,
+                final_context,
+                repeats,
+                warn_validation=False,
+                die_on_validate_error=die_on_validate_error,
             )
 
             final_exp_name = app_inst.expander.expand_var_name(self.keywords.experiment_namespace)
@@ -477,7 +486,12 @@ def _ingest_experiments(self):
             render_group, exclude_where=exclude_where
         ):
             app_inst = self._prepare_experiment(
-                experiment_template_name, experiment_vars, final_context, repeats
+                experiment_template_name,
+                experiment_vars,
+                final_context,
+                repeats,
+                warn_validation=False,
+                die_on_validate_error=die_on_validate_error,
             )
 
             final_exp_name = app_inst.expander.expand_var_name(self.keywords.experiment_name)
@@ -526,11 +540,15 @@ def _ingest_experiments(self):
                     logger.die(f"Experiment {final_exp_namespace} is not unique.")
 
                 try:
-                    self.keywords.check_required_keys(experiment_vars)
-                except ramble.keywords.RambleKeywordError as e:
-                    raise RambleVariableDefinitionError(
-                        f"In experiment {final_exp_namespace}: {e}"
+                    app_inst.validate_experiment(
+                        warn_validation=True, die_on_validate_error=die_on_validate_error
                     )
+                except ramble.keywords.RambleKeywordError as e:
+                    if die_on_validate_error:
+                        raise RambleVariableDefinitionError(
+                            f"In experiment {final_exp_namespace}: {e}"
+                        )
+                    pass
 
                 rendered_experiments.add(final_exp_namespace)
                 self.experiments[final_exp_namespace] = app_inst

lib/ramble/ramble/keywords.py

@@ -168,7 +168,7 @@ def check_reserved_keys(self, definitions):
                     f'Keyword "{definition}" has been defined, ' + "but is reserved by ramble."
                 )
 
-    def check_required_keys(self, definitions):
+    def check_required_keys(self, definitions, warn_validation=True, die_on_validate_error=True):
         """Check a dictionary of variable definitions for all required keywords"""
         if not definitions:
             return
@@ -183,11 +183,14 @@ def check_required_keys(self, definitions):
                 required_set.remove(definition)
 
         if len(required_set) > 0:
-            for key in required_set:
-                logger.warn(f'Required key "{key}" is not defined')
-            raise RambleKeywordError(
-                "One or more required keys " + "are not defined within an experiment."
-            )
+            if warn_validation:
+                for key in required_set:
+                    logger.warn(f'Required key "{key}" is not defined')
+
+            if die_on_validate_error:
+                raise RambleKeywordError(
+                    "One or more required keys " + "are not defined within an experiment."
+                )
 
 
 class RambleKeywordError(ramble.error.RambleError):

lib/ramble/ramble/modkit.py

@@ -7,8 +7,7 @@
 # except according to those terms.
 
 # flake8: noqa: F401
-"""modkit is a set of useful modules to import when writing modifiers
-"""
+"""modkit is a set of useful modules to import when writing modifiers"""
 
 import llnl.util.filesystem
 from llnl.util.filesystem import *

lib/ramble/ramble/pkgmankit.py

@@ -7,8 +7,7 @@
 # except according to those terms.
 
 # flake8: noqa: F401
-"""pkgmankit is a set of useful modules to import when writing package managers
-"""
+"""pkgmankit is a set of useful modules to import when writing package managers"""
 
 import os
 

lib/ramble/ramble/test/concretize_builtin.py

@@ -16,6 +16,7 @@
 # everything here uses the mock_workspace_path
 pytestmark = pytest.mark.usefixtures("mutable_config", "mutable_mock_workspace_path")
 
+config = RambleCommand("config")
 workspace = RambleCommand("workspace")
 
 
@@ -97,3 +98,45 @@ def test_concretize_does_not_set_required(mutable_config, mutable_mock_workspace
                     req_test = False
                     break
             assert req_test
+
+
+def test_concretize_allows_invalid_experiment(
+    mutable_config, mutable_mock_workspace_path, request
+):
+    ws_name = request.node.name
+
+    global_args = ["-w", ws_name]
+
+    with ramble.workspace.create(ws_name) as ws:
+
+        workspace(
+            "manage",
+            "experiments",
+            "gromacs",
+            "--wf",
+            "water_bare",
+            "-p",
+            "spack",
+            global_args=global_args,
+        )
+
+        # Remove variables, to create an invalid experiment
+        config(
+            "rm",
+            "applications:gromacs:workloads:water_bare:experiments:generated:n_ranks",
+            global_args=global_args,
+        )
+        config(
+            "rm",
+            "applications:gromacs:workloads:water_bare:experiments:generated:n_nodes",
+            global_args=global_args,
+        )
+
+        ws._re_read()
+
+        workspace("concretize")
+
+        with open(ws.config_file_path) as f:
+            data = f.read()
+
+            assert "spack_gromacs" in data

lib/ramble/ramble/wmkit.py

@@ -7,8 +7,7 @@
 # except according to those terms.
 
 # flake8: noqa: F401
-"""wmkit is a set of useful modules to import when writing workflow managers
-"""
+"""wmkit is a set of useful modules to import when writing workflow managers"""
 
 from ramble.language.shared_language import *
 from ramble.language.workflow_manager_language import *

lib/ramble/ramble/workspace/__init__.py

@@ -6,8 +6,7 @@
 # option. This file may not be copied, modified, or distributed
 # except according to those terms.
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
-"""This package implements Ramble workspaces.
-"""
+"""This package implements Ramble workspaces."""
 
 from .workspace import (
     RambleActiveWorkspaceError,

lib/ramble/ramble/workspace/workspace.py

@@ -844,7 +844,7 @@ def all_specs(self):
     def all_experiments_path(self):
         return os.path.join(self.root, workspace_all_experiments_file)
 
-    def build_experiment_set(self):
+    def build_experiment_set(self, die_on_validate_error=True):
         """Create an experiment set representing this workspace"""
 
         experiment_set = ramble.experiment_set.ExperimentSet(self)
@@ -858,7 +858,9 @@ def build_experiment_set(self):
                 experiment_set.set_workload_context(workload_context)
 
                 for _, experiment_context in self.all_experiments(experiments):
-                    experiment_set.set_experiment_context(experiment_context)
+                    experiment_set.set_experiment_context(
+                        experiment_context, die_on_validate_error=die_on_validate_error
+                    )
 
         experiment_set.build_experiment_chains()
 
@@ -1395,7 +1397,7 @@ def concretize(self, force=False, quiet=False):
 
         self.software_environments = ramble.software_environments.SoftwareEnvironments(self)
 
-        experiment_set = self.build_experiment_set()
+        experiment_set = self.build_experiment_set(die_on_validate_error=False)
 
         for _, app_inst, _ in experiment_set.all_experiments():
             app_inst.build_modifier_instances()