From 83360cf227d707f47893776ba8086b24312427f0 Mon Sep 17 00:00:00 2001 From: marcosf2 Date: Tue, 11 Aug 2026 23:05:00 -0500 Subject: [PATCH] Refactored `analyze` to platform-specific hooks with a default fallback; added tests and updated documentation. --- CONTEXT.md | 4 +- docs/user_guide/protocols/index.md | 6 +- docs/user_guide/protocols/operations.md | 60 +++++++++----- src/labcore/protocols/base.py | 29 ++++++- src/labcore/testing/protocol_dummy/cosine.py | 2 +- .../testing/protocol_dummy/exponential.py | 2 +- .../protocol_dummy/exponential_decay.py | 2 +- .../exponentially_decaying_sine.py | 2 +- .../testing/protocol_dummy/gaussian.py | 2 +- .../gaussian_with_correction.py | 2 +- src/labcore/testing/protocol_dummy/linear.py | 2 +- test/pytest/test_protocols.py | 81 +++++++++++++++++-- 12 files changed, 155 insertions(+), 39 deletions(-) diff --git a/CONTEXT.md b/CONTEXT.md index 9a1e640..9e293b5 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -56,7 +56,9 @@ clarified; remove or rewrite entries that go stale. - **Platform** — the hardware backend a protocol runs against (`DUMMY`, `QICK`, `OPX`). Selected globally via the `PLATFORMTYPE` module variable in `labcore.protocols.base`; parameters and operations dispatch to - platform-specific code (`_dummy_getter`, `_qick_getter`, …) based on it. + platform-specific code (`_dummy_getter`, `_measure_qick`, …) based on it. + Analysis uses a shared default unless an operation supplies a + platform-specific analysis. - **Report** — a self-contained HTML document assembled by `ProtocolBase._assemble_report()` after a protocol runs. Each operation diff --git a/docs/user_guide/protocols/index.md b/docs/user_guide/protocols/index.md index 4181a61..90e314d 100644 --- a/docs/user_guide/protocols/index.md +++ b/docs/user_guide/protocols/index.md @@ -38,7 +38,7 @@ super-operations, conditions, and the assembled report. Every operation runs the same five steps in order, on every attempt: ``` - ◀── platform-specific ──▶ ◀───── platform-agnostic ──────▶ + ◀── platform-specific ──▶ ◀ optional override ▶ ◀─ platform-agnostic ─▶ measure ──▶ load_data ──▶ analyze ──▶ evaluate ──▶ correct │ │ │ │ │ @@ -51,8 +51,8 @@ Every operation runs the same five steps in order, on every attempt: ``` - `measure` — performs the measurement (or generates fake data on `DUMMY`) and saves the raw data to disk. -- `load_data` — reads the raw data back into memory and normalizes its shape and field names so the rest of the lifecycle is platform-agnostic. -- `analyze` — runs fits and statistics over the loaded data and attaches the results to the operation. +- `load_data` — reads the raw data back into memory and normalizes its shape and field names so later steps can usually be shared across platforms. +- `analyze` — runs fits and statistics over the loaded data and attaches the results to the operation. It uses the default analysis unless the selected platform has a specialized implementation. - `evaluate` — returns named check results and an overall status; pure assessment, no side effects. - `correct` — the only place parameters get written: fitted outputs on success, a correction strategy on retry. diff --git a/docs/user_guide/protocols/operations.md b/docs/user_guide/protocols/operations.md index 604fbf3..6f6804e 100644 --- a/docs/user_guide/protocols/operations.md +++ b/docs/user_guide/protocols/operations.md @@ -13,7 +13,7 @@ This page assumes you have read {doc}`parameters`. ## The lifecycle of an operation ``` - ◀── platform-specific ──▶ ◀───── platform-agnostic ──────▶ + ◀── platform-specific ──▶ ◀ optional override ▶ ◀─ platform-agnostic ─▶ measure ──▶ load_data ──▶ analyze ──▶ evaluate ──▶ correct │ │ │ │ │ @@ -26,10 +26,11 @@ This page assumes you have read {doc}`parameters`. ``` The split between platform-specific and platform-agnostic steps is -deliberate: `analyze`, `evaluate`, and `correct` should run identically no -matter which backend produced the data. Whatever per-platform quirks exist -in field names, units, or array shapes have to be reconciled by -`load_data` so that everything downstream sees a single canonical shape. +deliberate: `evaluate` and `correct` should run identically no matter which +backend produced the data. `analyze` should normally be shared too, but it +can specialize for a platform when the analysis itself genuinely differs. +Per-platform quirks in field names and array shapes should still be +reconciled by `load_data` so later steps see a canonical structure. - **`measure`** performs the measurement (or generates fake data on `DUMMY`) and saves it to disk via the standard sweep + DDH5 machinery. @@ -39,12 +40,17 @@ in field names, units, or array shapes have to be reconciled by data so that downstream steps see the same shape and variable names regardless of platform**. Different backends can save data with different field names or slightly different shapes; reconciling those - differences here is what lets `analyze` be platform-agnostic. Stores + differences here is what lets analysis use a shared default in most + operations. Stores the result on the operation as `independents` and `dependents` dictionaries. Dispatches to `_load_data_dummy` / `_load_data_qick` / `_load_data_opx`. -- **`analyze`** is platform-agnostic. Run your fits, compute summary - statistics, attach results to `self`. Do **not** mutate parameters here. +- **`analyze`** dispatches to `_analyze_dummy`, `_analyze_qick`, or + `_analyze_opx`. Each inherited platform hook falls back to + `_analyze_default`, so most operations only implement that default. + Override a platform hook only when its analysis genuinely differs. Run + fits, compute summary statistics, and attach results to `self`; do + **not** mutate parameters here. - **`evaluate`** is **pure assessment**. It returns named check results and an overall status (`SUCCESS` / `RETRY` / `FAILURE`). No side effects. By default this just runs every check registered with @@ -115,8 +121,9 @@ an attribute on the operation. After the calls above, `self.center()`, get verified before the protocol runs; outputs are written by `correct()` on success; correction parameters skip the hardware verification check. -Platform-specific work — measurement and data loading — is split exactly -the way parameter getters and setters are: +Platform-specific work is split into hooks in the same way as parameter +getters and setters. Measurement and data loading require a hook for each +supported platform; analysis additionally provides a shared default: ```python def _measure_dummy(self) -> Path: @@ -131,13 +138,28 @@ def _load_data_dummy(self) -> None: data = datadict_from_hdf5(self.data_loc / "data.ddh5") self.independents["x_values"] = data["x"]["values"] self.dependents["y_values"] = data["y"]["values"] + +def _analyze_default(self) -> None: + # shared fitting and statistics for every platform without an override + ... + +def _analyze_qick(self) -> None: + # optional: replace the default only when QICK analysis genuinely differs + ... ``` -The base class's `measure()` and `load_data()` dispatch to the right -method based on the platform selected with +The base class's `measure()`, `load_data()`, and `analyze()` dispatch to the +right method based on the platform selected with {py:func}`select_platform `. You only -implement the platforms you actually run on; the others raise -`NotImplementedError` if invoked. +implement measurement and loading for platforms you actually run on; the +others raise `NotImplementedError` if invoked. For analysis, an omitted +platform hook automatically calls `_analyze_default`. A platform hook is a +complete replacement, but it can call `_analyze_default()` explicitly when +it only needs to add behavior. + +Treat `analyze()` as the public lifecycle method called by the framework. +Normal operations should override `_analyze_default()` or one of the +platform hooks rather than overriding `analyze()` itself. :::{note} The leading underscore on methods like `_register_inputs`, @@ -148,7 +170,7 @@ protocol, and let the framework call these for you. Whoever is **writing** an operation absolutely does use them — in `__init__` and in overrides. The same convention applies everywhere on this page (`_register_outputs`, `_register_correction_params`, `_register_check`, -`_register_success_update`, `_measure_*`, `_load_data_*`, …). +`_register_success_update`, `_measure_*`, `_load_data_*`, `_analyze_*`, …). ::: ## Correcting itself @@ -360,8 +382,8 @@ inline and the lmfit fit report dumped in a code block — all written by Here is a complete, runnable operation that uses every concept introduced above — a registered output, a registered check, a registered success -update, platform-specific `measure` and `load_data`, and a -platform-agnostic `analyze`. Copy it into a script, run it, and the +update, platform-specific `measure` and `load_data`, and a default analysis. +Copy it into a script, run it, and the protocol will execute end-to-end on the `DUMMY` platform: ```python @@ -417,7 +439,7 @@ class MinimalGaussianFit(ProtocolOperation): self.independents["x_values"] = data["x"]["values"] self.dependents["y_values"] = data["y"]["values"] - def analyze(self) -> None: + def _analyze_default(self) -> None: with DatasetAnalysis(self.data_loc, self.name) as ds: x = np.asarray(self.independents["x_values"]) y = np.asarray(self.dependents["y_values"]) @@ -469,7 +491,7 @@ That file maps onto the sections of this page like so: | Registering a check + correction | `_register_check` call in `__init__` | | Correction subclass | `_ReduceNoiseLevelCorrection` | | Platform code | `_measure_dummy`, `_load_data_dummy` | -| Analyze | `analyze()` | +| Default analysis | `_analyze_default()` | | Override of `correct()` | bottom of the class | ## Where to read next diff --git a/src/labcore/protocols/base.py b/src/labcore/protocols/base.py index 66f5272..3ddf73e 100644 --- a/src/labcore/protocols/base.py +++ b/src/labcore/protocols/base.py @@ -473,8 +473,35 @@ def measure(self) -> Path: return loc raise NotImplementedError(f"Platform type {self.platform_type} not implemented") + def _analyze_default(self) -> None: + """Analyze loaded data when no platform-specific implementation exists.""" + raise NotImplementedError("Default analysis not implemented") + + def _analyze_qick(self) -> None: + """Analyze QICK data, falling back to the default implementation.""" + self._analyze_default() + + def _analyze_opx(self) -> None: + """Analyze OPX data, falling back to the default implementation.""" + self._analyze_default() + + def _analyze_dummy(self) -> None: + """Analyze DUMMY data, falling back to the default implementation.""" + self._analyze_default() + def analyze(self) -> None: - raise NotImplementedError("Analyze method not implemented") + """Dispatch analysis to the selected platform, with a default fallback.""" + match self.platform_type: + case PlatformTypes.QICK: + self._analyze_qick() + case PlatformTypes.OPX: + self._analyze_opx() + case PlatformTypes.DUMMY: + self._analyze_dummy() + case _: + raise NotImplementedError( + f"Platform type {self.platform_type} not implemented" + ) def _load_data_opx(self) -> None: raise NotImplementedError("Load OPX data method not implemented") diff --git a/src/labcore/testing/protocol_dummy/cosine.py b/src/labcore/testing/protocol_dummy/cosine.py index 0a973ff..2c7b3b9 100644 --- a/src/labcore/testing/protocol_dummy/cosine.py +++ b/src/labcore/testing/protocol_dummy/cosine.py @@ -101,7 +101,7 @@ def _load_data_dummy(self) -> None: self.independents["x_values"] = data["x"]["values"] self.dependents["y_values"] = data["y"]["values"] - def analyze(self) -> None: + def _analyze_default(self) -> None: """Fit the data to a Cosine""" assert self.data_loc is not None with DatasetAnalysis(self.data_loc, self.name) as ds: diff --git a/src/labcore/testing/protocol_dummy/exponential.py b/src/labcore/testing/protocol_dummy/exponential.py index 3a0b399..b7f2476 100644 --- a/src/labcore/testing/protocol_dummy/exponential.py +++ b/src/labcore/testing/protocol_dummy/exponential.py @@ -90,7 +90,7 @@ def _load_data_dummy(self) -> None: self.independents["x_values"] = data["x"]["values"] self.dependents["y_values"] = data["y"]["values"] - def analyze(self) -> None: + def _analyze_default(self) -> None: """Fit the data to an Exponential""" assert self.data_loc is not None with DatasetAnalysis(self.data_loc, self.name) as ds: diff --git a/src/labcore/testing/protocol_dummy/exponential_decay.py b/src/labcore/testing/protocol_dummy/exponential_decay.py index 2edacf4..fb7083c 100644 --- a/src/labcore/testing/protocol_dummy/exponential_decay.py +++ b/src/labcore/testing/protocol_dummy/exponential_decay.py @@ -97,7 +97,7 @@ def _load_data_dummy(self) -> None: self.independents["x_values"] = data["x"]["values"] self.dependents["y_values"] = data["y"]["values"] - def analyze(self) -> None: + def _analyze_default(self) -> None: """Fit the data to an Exponential Decay""" assert self.data_loc is not None with DatasetAnalysis(self.data_loc, self.name) as ds: diff --git a/src/labcore/testing/protocol_dummy/exponentially_decaying_sine.py b/src/labcore/testing/protocol_dummy/exponentially_decaying_sine.py index cc3534f..b5fc1b2 100644 --- a/src/labcore/testing/protocol_dummy/exponentially_decaying_sine.py +++ b/src/labcore/testing/protocol_dummy/exponentially_decaying_sine.py @@ -111,7 +111,7 @@ def _load_data_dummy(self) -> None: self.independents["x_values"] = data["x"]["values"] self.dependents["y_values"] = data["y"]["values"] - def analyze(self) -> None: + def _analyze_default(self) -> None: """Fit the data to an Exponentially Decaying Sine""" assert self.data_loc is not None with DatasetAnalysis(self.data_loc, self.name) as ds: diff --git a/src/labcore/testing/protocol_dummy/gaussian.py b/src/labcore/testing/protocol_dummy/gaussian.py index 9a8b239..7f6fe25 100644 --- a/src/labcore/testing/protocol_dummy/gaussian.py +++ b/src/labcore/testing/protocol_dummy/gaussian.py @@ -99,7 +99,7 @@ def _load_data_dummy(self) -> None: self.independents["x_values"] = data["x"]["values"] self.dependents["y_values"] = data["y"]["values"] - def analyze(self) -> None: + def _analyze_default(self) -> None: """Fit the data to a Gaussian""" assert self.data_loc is not None with DatasetAnalysis(self.data_loc, self.name) as ds: diff --git a/src/labcore/testing/protocol_dummy/gaussian_with_correction.py b/src/labcore/testing/protocol_dummy/gaussian_with_correction.py index 53a7bd0..f0d9ea7 100644 --- a/src/labcore/testing/protocol_dummy/gaussian_with_correction.py +++ b/src/labcore/testing/protocol_dummy/gaussian_with_correction.py @@ -185,7 +185,7 @@ def _load_data_dummy(self) -> None: self.independents["x_values"] = data["x"]["values"] self.dependents["y_values"] = data["y"]["values"] - def analyze(self) -> None: + def _analyze_default(self) -> None: assert self.data_loc is not None with DatasetAnalysis(self.data_loc, self.name) as ds: x = np.asarray(self.independents["x_values"]) diff --git a/src/labcore/testing/protocol_dummy/linear.py b/src/labcore/testing/protocol_dummy/linear.py index 307cd3b..88bdad3 100644 --- a/src/labcore/testing/protocol_dummy/linear.py +++ b/src/labcore/testing/protocol_dummy/linear.py @@ -87,7 +87,7 @@ def _load_data_dummy(self) -> None: self.independents["x_values"] = data["x"]["values"] self.dependents["y_values"] = data["y"]["values"] - def analyze(self) -> None: + def _analyze_default(self) -> None: """Fit the data to a Linear function""" assert self.data_loc is not None with DatasetAnalysis(self.data_loc, self.name) as ds: diff --git a/test/pytest/test_protocols.py b/test/pytest/test_protocols.py index 6e58efc..0dc7416 100644 --- a/test/pytest/test_protocols.py +++ b/test/pytest/test_protocols.py @@ -81,7 +81,7 @@ def _measure_dummy(self): def _load_data_dummy(self): log.append("load_data") - def analyze(self): + def _analyze_default(self): log.append("analyze") def evaluate(self) -> EvaluateResult: @@ -185,6 +185,71 @@ def test_execute_calls_workflow_in_order(self): op.execute() assert log == ["measure", "load_data", "analyze", "evaluate"] + @pytest.mark.parametrize( + ("platform", "expected"), + [ + (PlatformTypes.QICK, "qick"), + (PlatformTypes.OPX, "opx"), + (PlatformTypes.DUMMY, "dummy"), + ], + ) + def test_analyze_dispatches_to_platform_implementation(self, platform, expected): + calls = [] + + class _Op(ProtocolOperation): + def _analyze_default(self): + calls.append("default") + + def _analyze_qick(self): + calls.append("qick") + + def _analyze_opx(self): + calls.append("opx") + + def _analyze_dummy(self): + calls.append("dummy") + + op = _Op() + op.platform_type = platform + op.analyze() + + assert calls == [expected] + + @pytest.mark.parametrize( + "platform", [PlatformTypes.QICK, PlatformTypes.OPX, PlatformTypes.DUMMY] + ) + def test_analyze_falls_back_to_default(self, platform): + calls = [] + + class _Op(ProtocolOperation): + def _analyze_default(self): + calls.append("default") + + op = _Op() + op.platform_type = platform + op.analyze() + + assert calls == ["default"] + + def test_analyze_raises_when_no_implementation_exists(self): + with pytest.raises(NotImplementedError, match="Default analysis"): + ProtocolOperation().analyze() + + def test_analyze_does_not_mask_errors_from_platform_implementation(self): + calls = [] + + class _Op(ProtocolOperation): + def _analyze_default(self): + calls.append("default") + + def _analyze_dummy(self): + raise NotImplementedError("nested analysis is unavailable") + + with pytest.raises(NotImplementedError, match="nested analysis"): + _Op().analyze() + + assert calls == [] + def test_execute_increments_attempt_counters(self): op, _ = make_simple_op() op.execute() @@ -274,7 +339,7 @@ def _measure_dummy(self): def _load_data_dummy(self): pass - def analyze(self): + def _analyze_default(self): pass def evaluate(self) -> EvaluateResult: @@ -451,7 +516,7 @@ def _measure_dummy(self): def _load_data_dummy(self): pass - def analyze(self): + def _analyze_default(self): pass def evaluate(self) -> EvaluateResult: @@ -484,7 +549,7 @@ def _measure_dummy(self): def _load_data_dummy(self): pass - def analyze(self): + def _analyze_default(self): pass def evaluate(self) -> EvaluateResult: @@ -511,7 +576,7 @@ def _measure_dummy(self): def _load_data_dummy(self): pass - def analyze(self): + def _analyze_default(self): pass def evaluate(self) -> EvaluateResult: @@ -541,7 +606,7 @@ def _measure_dummy(self): def _load_data_dummy(self): pass - def analyze(self): + def _analyze_default(self): pass def evaluate(self) -> EvaluateResult: @@ -619,7 +684,7 @@ def _measure_dummy(self): def _load_data_dummy(self): pass - def analyze(self): + def _analyze_default(self): pass def evaluate(self) -> EvaluateResult: @@ -705,7 +770,7 @@ def _measure_dummy(self): def _load_data_dummy(self): pass - def analyze(self): + def _analyze_default(self): pass return _Op()