Skip to content

Commit f93cb69

Browse files
Fix defining workflow and package manager variables
Preivously, variables from workflow manager and package managers were not defined until the add_expand_vars method is called. This means they are defined quite late in the flow, and sometimes cause issues when validating experiments. This commit moves these definitions to when package managers and workflow managers are added to the experiment.
1 parent e42c136 commit f93cb69

File tree

1 file changed

+45
-35
lines changed

1 file changed

+45
-35
lines changed

lib/ramble/ramble/application.py

Lines changed: 45 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -227,10 +227,12 @@ def clone(self):
227227
new_clone = type(self)(self._file_path)
228228
self.generated_experiments.append(new_clone)
229229

230-
if self._env_variable_sets:
231-
new_clone.set_env_variable_sets(self._env_variable_sets.copy())
232230
if self.variables:
233231
new_clone.set_variables(self.variables.copy(), self.experiment_set)
232+
if self.variants:
233+
new_clone.set_variants(self.variants)
234+
if self._env_variable_sets:
235+
new_clone.set_env_variable_sets(self._env_variable_sets.copy())
234236
if self.internals:
235237
new_clone.set_internals(self.internals.copy())
236238
if self._formatted_executables:
@@ -240,8 +242,6 @@ def clone(self):
240242
new_clone.set_template(False)
241243
new_clone.repeats.set_repeats(False, 0)
242244
new_clone.set_chained_experiments(None)
243-
if self.variants:
244-
new_clone.set_variants(self.variants)
245245

246246
return new_clone
247247

@@ -321,6 +321,11 @@ def _set_package_manager(self):
321321
}
322322
)
323323

324+
# Define any missing package manager variables
325+
for var, val in self.package_manager.selected_variables().items():
326+
if var not in self.variables:
327+
self.define_variable(var, val.default)
328+
324329
def _set_workflow_manager(self):
325330
workflow_name = conversions.canonical_none(
326331
self.object_variants.value(namespace.workflow_manager)
@@ -341,6 +346,12 @@ def _set_workflow_manager(self):
341346
"\tramble list --type workflow_managers"
342347
)
343348

349+
if self.workflow_manager is not None:
350+
# Define any missing workflow manager variables
351+
for var, val in self.workflow_manager.selected_variables().items():
352+
if var not in self.variables:
353+
self.define_variable(var, val.default)
354+
344355
def set_success_list(self, success_criteria):
345356
self.success_list = ramble.success_criteria.ScopedCriteriaList()
346357

@@ -386,6 +397,26 @@ def set_env_variable_sets(self, env_variable_sets):
386397

387398
self._env_variable_sets = env_variable_sets.copy()
388399

400+
# Extract workload environment variable sets
401+
if self.expander.workload_name in self.workloads:
402+
workload = self.workloads[self.expander.workload_name]
403+
404+
new_env_vars = {}
405+
for env_var in workload.environment_variables.values():
406+
action = "set"
407+
value = env_var.value
408+
409+
add = True
410+
for env_var_set in self._env_variable_sets:
411+
if action in env_var_set:
412+
if env_var.name in env_var_set[action].keys():
413+
add = False
414+
415+
if add:
416+
new_env_vars[env_var.name] = value
417+
418+
self._env_variable_sets.append({"set": new_env_vars})
419+
389420
def set_variables(self, variables, experiment_set):
390421
"""Set internal reference to variables
391422
@@ -405,6 +436,11 @@ def set_variables(self, variables, experiment_set):
405436
if not var.expandable:
406437
self.no_expand_vars.add(var.name)
407438

439+
# Define missing workload variables
440+
for var, val in self.selected_variables().items():
441+
if var not in self.variables:
442+
self.define_variable(var, val.default)
443+
408444
self.expander.set_no_expand_vars(self.no_expand_vars)
409445

410446
def set_internals(self, internals):
@@ -928,6 +964,11 @@ def build_modifier_instances(self):
928964
self.expander.add_no_expand_var(var)
929965
mod_inst.expander.add_no_expand_var(var)
930966

967+
# Define any missing modifier variables
968+
for var, val in mod_inst.selected_variables().items():
969+
if var not in self.variables:
970+
self.define_variable(var, val.default)
971+
931972
# Set standard variants for all modifiers
932973
obj_variants = getattr(mod_inst, "object_variants", None)
933974
if obj_variants is not None:
@@ -1083,36 +1124,6 @@ def selected_variables(self):
10831124

10841125
return wl_vars
10851126

1086-
def _set_default_experiment_variables(self):
1087-
"""Set default experiment variables (for add_expand_vars),
1088-
if they haven't been set already"""
1089-
# Set default experiment variables, if they haven't been set already
1090-
1091-
# Define variables from var_sets
1092-
for _, obj in self._objects():
1093-
for var, val in obj.selected_variables().items():
1094-
if var not in self.variables:
1095-
self.define_variable(var, val.default)
1096-
1097-
if self.expander.workload_name in self.workloads:
1098-
workload = self.workloads[self.expander.workload_name]
1099-
1100-
new_env_vars = {}
1101-
for env_var in workload.environment_variables.values():
1102-
action = "set"
1103-
value = env_var.value
1104-
1105-
add = True
1106-
for env_var_set in self._env_variable_sets:
1107-
if action in env_var_set:
1108-
if env_var.name in env_var_set[action].keys():
1109-
add = False
1110-
1111-
if add:
1112-
new_env_vars[env_var.name] = value
1113-
1114-
self._env_variable_sets.append({"set": new_env_vars})
1115-
11161127
def _define_commands(self, exec_graph, success_list=None):
11171128
"""Populate the internal list of commands based on executables
11181129
@@ -1328,7 +1339,6 @@ def add_expand_vars(self, workspace):
13281339
if not self._vars_are_expanded:
13291340
self._validate_experiment()
13301341
self._executable_graph = self._get_executable_graph(self.expander.workload_name)
1331-
self._set_default_experiment_variables()
13321342
self._set_input_path()
13331343

13341344
self._derive_variables_for_template_path(workspace)

0 commit comments

Comments
 (0)