allow expressions in config overrides

Author: CamDavidsonPilon

.agents/skills/pioreactor-experiment-profiles/SKILL.md

@@ -63,7 +63,7 @@ pioreactors:
 #   if: <bool_or_expression>
 #   options: {<option_name>: <value>}  # expressions allowed via ${{ }}
 #   args: [<string>, ...]
-#   config_overrides: {<config_name>: <value>}
+#   config_overrides: {<config_name>: <value>}  # expressions allowed via ${{ }}
 #
 # - type: update
 #   t: <time_string_or_float>

core/pioreactor/actions/leader/experiment_profile.py

@@ -725,7 +725,7 @@ def start_job(
     action_metrics: ActionMetrics,
     options: dict[str, Any],
     args: list[str],
-    config_overrides: dict[str, str],
+    config_overrides: dict[str, Any],
 ) -> Callable[..., None]:
     def _callable() -> None:
         nonlocal env
@@ -741,13 +741,15 @@ def _callable() -> None:
             return
 
         if dry_run:
+            evaluated_config_overrides = evaluate_options(config_overrides, env)
             logger.info(
-                f"{action_count}. Dry-run: Starting {job_name} on {unit} with options {evaluate_options(options, env)} and args {args}."
+                f"{action_count}. Dry-run: Starting {job_name} on {unit} with options {evaluate_options(options, env)} and args {args}, and overrides {evaluated_config_overrides}."
             )
         else:
             evaluated_options = evaluate_options(options, env)
+            evaluated_config_overrides = evaluate_options(config_overrides, env)
             logger.debug(
-                f"{action_count}. Starting {job_name} on {unit} with options {evaluated_options}, args {args}, and overrides {config_overrides}."
+                f"{action_count}. Starting {job_name} on {unit} with options {evaluated_options}, args {args}, and overrides {evaluated_config_overrides}."
             )
             address = resolve_to_address(unit)
             for attempt in range(1, START_JOB_SUBMIT_MAX_ATTEMPTS + 1):
@@ -760,8 +762,8 @@ def _callable() -> None:
                             "env": _get_worker_env_for_start(unit, experiment, parent_job.job_key),
                             "args": args,
                             "config_overrides": [
-                                [f"{job_name}.config", key, value]
-                                for (key, value) in config_overrides.items()
+                                [f"{job_name}.config", key, str(value)]
+                                for (key, value) in evaluated_config_overrides.items()
                             ],
                         },
                     )

core/pioreactor/experiment_profiles/profiles_spec.txt

@@ -49,7 +49,7 @@ pioreactors:
   if: <bool_or_expression>
   options: {<option_name>: <value>}  # expressions allowed via ${{ }}
   args: [<string>, ...]
-  config_overrides: {<config_name>: <value>}
+  config_overrides: {<config_name>: <value>}  # expressions allowed via ${{ }}
 
 - type: update
   t: <time_string_or_float>

core/pioreactor/experiment_profiles/validate.py

@@ -203,6 +203,18 @@ def _validate_options_expressions(
             )
 
 
+def _validate_config_override_expressions(
+    diagnostics: list[Diagnostic], *, path: str, config_overrides: dict[str, Any]
+) -> None:
+    for key, value in config_overrides.items():
+        if is_bracketed_expression(value):
+            _validate_expression_field(
+                diagnostics,
+                path=f"{path}.config_overrides.{key}",
+                expression=value,
+            )
+
+
 def _validate_log_message_expressions(diagnostics: list[Diagnostic], *, path: str, message: str) -> None:
     for match in re.findall(FLEXIBLE_EXPRESSION_PATTERN, message):
         _validate_expression_field(
@@ -369,6 +381,11 @@ def _validate_action_expressions(diagnostics: list[Diagnostic], *, path: str, ac
     if isinstance(action, (struct.Start, struct.Update)):
         _validate_options_expressions(diagnostics, path=path, options=action.options)
 
+    if isinstance(action, struct.Start):
+        _validate_config_override_expressions(
+            diagnostics, path=path, config_overrides=action.config_overrides
+        )
+
     if isinstance(action, struct.Log):
         _validate_log_message_expressions(diagnostics, path=path, message=action.options.message)
 

core/tests/test_execute_experiment_profile.py

@@ -173,6 +173,40 @@ def test_execute_experiment_profile_hack_for_led_intensity(
     }
 
 
+@patch("pioreactor.actions.leader.experiment_profile._load_experiment_profile")
+def test_execute_experiment_profile_start_evaluates_config_overrides(
+    mock__load_experiment_profile,
+) -> None:
+    experiment = "_testing_experiment"
+    profile = Profile(
+        experiment_profile_name="test_profile",
+        inputs={"dc": 100},
+        pioreactors={
+            "unit1": PioreactorSpecificBlock(
+                jobs={
+                    "air_bubbler": Job(
+                        actions=[
+                            Start(
+                                t="0s",
+                                config_overrides={"duty_cycle": "${{dc}}"},
+                            )
+                        ]
+                    )
+                }
+            )
+        },
+        metadata=Metadata(author="test_author"),
+    )
+
+    mock__load_experiment_profile.return_value = profile
+
+    with capture_requests() as bucket:
+        execute_experiment_profile("profile.yaml", experiment)
+
+    assert bucket[0].url == "http://unit1.local:4999/unit_api/jobs/run/job_name/air_bubbler"
+    assert bucket[0].json["config_overrides"] == [["air_bubbler.config", "duty_cycle", "100"]]
+
+
 @patch("pioreactor.actions.leader.experiment_profile._load_experiment_profile")
 def test_execute_experiment_profile_start_failure_is_logged(
     mock__load_experiment_profile, caplog: pytest.LogCaptureFixture
@@ -1568,7 +1602,7 @@ def test_execute_experiment_profile_with_config_overrides(
     action = Start(
         hours_elapsed=0,
         options={"target": "${{unit1:jobbing:target + 1}}", "dont_eval": "1.0 + 1.0"},
-        config_overrides={"option1": "value1", "option2": "value2"},
+        config_overrides={"option1": "value1", "option2": "value2", "option3": 100},
     )
 
     profile = Profile(
@@ -1604,5 +1638,6 @@ def test_execute_experiment_profile_with_config_overrides(
         "config_overrides": [
             ["jobbing.config", "option1", "value1"],
             ["jobbing.config", "option2", "value2"],
+            ["jobbing.config", "option3", "100"],
         ],
     }

core/tests/test_validate_experiment_profile.py

@@ -117,6 +117,10 @@ def test_validate_profile_checks_all_expression_fields_with_full_parser() -> Non
           t: 0s
           options:
             target_rpm: "${{ 1 + }}"
+        - type: start
+          t: 0s
+          config_overrides:
+            target_rpm: "${{ 1 + }}"
         - type: log
           t: 0s
           options:
@@ -136,7 +140,8 @@ def test_validate_profile_checks_all_expression_fields_with_full_parser() -> Non
         "common.jobs.stirring.actions[2].condition",
         "common.jobs.stirring.actions[3].wait_until",
         "common.jobs.stirring.actions[4].options.target_rpm",
-        "common.jobs.stirring.actions[5].options.message",
+        "common.jobs.stirring.actions[5].config_overrides.target_rpm",
+        "common.jobs.stirring.actions[6].options.message",
     }
 
 

frontend/src/__tests__/ExperimentProfileEditor.test.jsx

@@ -206,4 +206,28 @@ pioreactors:
 
     expect(screen.getAllByText("??").length).toBeGreaterThan(0);
   });
+
+  test("shows invalid config overrides syntax when config overrides are written as a list", () => {
+    renderEditor({
+      initialCode: `experiment_profile_name: preview
+
+pioreactors:
+  xr1:
+    jobs:
+      stirring:
+        actions:
+          - type: update
+            t: 0s
+            options:
+              target_rpm: 500
+            config_overrides:
+              - target_rpm: 500
+`,
+      initialFilename: "draft_profile",
+      filenameEditable: true,
+      onSave: async () => {},
+    });
+
+    expect(screen.getByText("invalid config overrides syntax!")).toBeInTheDocument();
+  });
 });

frontend/src/components/DisplayProfile.jsx

@@ -385,6 +385,17 @@ const ActionDetails = ({ action, jobName, index, actionPath, comments }) => {
     return null;
   };
 
+  const renderInvalidConfigOverridesMessage = () => {
+    if (Array.isArray(action.config_overrides)) {
+      return (
+        <Typography variant="body2" component="div" sx={level3}>
+          <UnderlineSpan title="`config_overrides` field doesn't use `-` in front. Remove it.">invalid config overrides syntax!</UnderlineSpan>
+        </Typography>
+      );
+    }
+    return null;
+  };
+
   switch (action?.type) {
     case 'start':
     case 'update':
@@ -396,6 +407,7 @@ const ActionDetails = ({ action, jobName, index, actionPath, comments }) => {
           {renderOptions(action?.type)}
           {renderInvalidOptionsMessage()}
           {renderConfigOverrides()}
+          {renderInvalidConfigOverridesMessage()}
         </>
       );
     case 'log':