Skip to content
This repository was archived by the owner on Aug 12, 2026. It is now read-only.

fix(experiments): harden optional Python experiment lifecycle - #113

Merged
acgetchell merged 1 commit into
mainfrom
fix/107-python-experiment-import-safety
Jul 21, 2026
Merged

acgetchell merged 1 commit into
mainfrom
fix/107-python-experiment-import-safety

Conversation

@acgetchell

@acgetchell acgetchell commented Jul 21, 2026 •

Copy link
Copy Markdown
Owner
  • record geometry parameters for every initializer optimization run
  • isolate initializer and Comet boundaries from import-time behavior
  • close each Comet experiment after successful or failed execution

Fixes #107

Summary by CodeRabbit

  • Refactor

    • Improved initializer optimization reliability and flexibility.
    • Parameter sweeps now record geometry settings for each experiment.
    • Experiments are properly closed even when an initialization run fails.
  • Testing

    • Added coverage to verify parameter logging and cleanup behavior.
    • Added checks that optional experiment scripts can be imported without launching experiments or loading heavy dependencies.

- record geometry parameters for every initializer optimization run
- isolate initializer and Comet boundaries from import-time behavior
- close each Comet experiment after successful or failed execution

Fixes #107
@acgetchell acgetchell self-assigned this Jul 21, 2026
@coderabbitai

coderabbitai Bot commented Jul 21, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 28bcb662-fd37-4cc8-8935-e9993e6a29dd

📥 Commits

Reviewing files that changed from the base of the PR and between 1d7269c and 760a59f.

📒 Files selected for processing (3)
  • scripts/optimize_initialize.py
  • scripts/tests/test_experiment_imports.py
  • scripts/tests/test_optimize_initialize.py

Walkthrough

The initializer optimization script now uses injected experiment, runner, and plotting interfaces, centralizes parameter pairs, records geometry inputs, and guarantees experiment finalization. New tests cover import safety, parameter logging, and cleanup on initializer failure.

Changes

Initializer sweep refactor

Layer / File(s) Summary
Injected sweep orchestration
scripts/optimize_initialize.py
Defines typed experiment and plotting boundaries, centralizes PARAMETER_PAIRS, uses foliation_spacing in command construction, logs sweep inputs, delegates execution, plots through the injected interface, and ends experiments in a finally block.
Import and lifecycle validation
scripts/tests/test_optimize_initialize.py, scripts/tests/test_experiment_imports.py
Tests verify optional scripts import without heavy dependencies or subprocesses, geometry parameters are logged, and experiments end after successful or failed initializer runs.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant _run_experiments
  participant _run_parameter_sweep
  participant initializer_runner
  participant _Plotter
  _run_experiments->>_run_parameter_sweep: provide experiment and runner factories
  _run_parameter_sweep->>initializer_runner: execute initializer command
  initializer_runner-->>_run_parameter_sweep: return initializer output
  _run_parameter_sweep->>_Plotter: plot and log figures
  _run_parameter_sweep->>_run_parameter_sweep: end experiment
Loading

Possibly related issues

  • Issue 104: The sweep lifecycle changes also explicitly end Comet experiments during optimization runs.

Poem

Parameters march in pairs,
Commands bloom from careful care,
Experiments end when runs are through,
Imports stay quiet, tests watch too.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly reflects the main change: hardening the optional experiment lifecycle.
Linked Issues check ✅ Passed The changes align with #107 by improving import safety, testability, parameter logging, and Comet cleanup with deterministic tests.
Out of Scope Changes check ✅ Passed The changes stay focused on experiment lifecycle hardening and related tests, with no clear unrelated additions.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/107-python-experiment-import-safety

Comment @coderabbitai help to get the list of available commands.

@acgetchell
acgetchell enabled auto-merge (squash) July 21, 2026 06:34
@acgetchell acgetchell added this to the v1.0.0-rc.1 milestone Jul 21, 2026
@acgetchell
acgetchell merged commit 31f95ab into main Jul 21, 2026
13 checks passed
@acgetchell
acgetchell deleted the fix/107-python-experiment-import-safety branch July 21, 2026 07:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make optional Python experiment scripts import-safe and testable

1 participant