Skip to content

mbr_campaign.STAGES names an aggregate class and a fixed program per stage; both are wrong #1

Description

@gaberber

Owner: me (guan). Not a task for anyone else -- filed so the reason is written
down somewhere other than my head, and so the fixes it makes unnecessary
(#2) can point at it.

The table

experiments/qsim/mbr_campaign.py:318-331:

STAGES = {
    "calibration":   (MBRPhaseCorrectionExperiment, EntireFloquetCyclePhaseCalibrationProgram, _calibration_batch),
    "spectrum":      (MBRSpectrumExperiment,        NPhotonHamiltonianSpectroscopyProgram,     _spectrum_batch),
    "propagator":    (MBRPropagatorExperiment,      EncodingPropagatorProgram,                 _propagator_batch),
    "orthogonality": (MBROrthogonalityExperiment,   EncodingOrthogonalityProgram,              _orthogonality_batch),
}

run_stage hands the first column straight to the acquisition runner
(mbr_campaign.py:357):

runner = BatchRunner(station=station, ExptClass=owner, ExptProgram=program, ...)

Both of the first two columns are wrong, and each has already produced a bug:

  • Column 1, the class. These are aggregate-analysis classes. Their
    analyze/display need batch_expts, which only the collected object has.
    Submitting them as ExptClass means the worker builds one per child job and
    calls the aggregate methods on a single 1D sweep. Patched at the symptom in
    0f9eddf (worker/runner flag) and a94880b (hasattr guards in display).
  • Column 2, the program. The program is not fixed per stage; for
    "spectrum" it depends on whether initial != final. Patched at the symptom
    in d47ab2f (batch.get("program", program)).

Two bugs in one week, from opposite directions, both from this one table. The
table claims a fixed (class, program) per stage, and neither is fixed.

Now vs. target

now target
class submitted as ExptClass the stage class always the job-sized class, EncodingHamiltonianSpectroscopyExperiment
program hardcoded per stage in STAGES taken from the batch object
what a stage class does acquires and aggregate-analyzes aggregate-analyzes only; no acquire, no program, never an ExptClass
how the two connect no seam; role picked at runtime by hasattr or a flag StageClass.from_batch(runner.execute(...))

Both target columns are already implemented elsewhere in the repo, which is
what makes this a gap and not an open design question:

  • experiments/qsim/notebook_helpers/mbr_campaign.py:299 and :504 already
    submit with ExptClass=campaign.EncSpec.
  • experiments/qsim/mbr_spectrum.py:1078 already shows
    ExptProgram=spectroscopy_batch.program.
  • from_batch (floquet_dark_mode_readout.py:75) exists only to turn an
    acquisition aggregate into a stage class. Its own docstring describes this
    seam. run_stage goes around it.

The archived surface map already recorded both decisions on 2026-09-14:
program_class is the stage key, and new MBR acquisitions record
EncodingHamiltonianSpectroscopyExperiment rather than a stage class. That
doc is marked superseded, so in practice the decision was invisible to anyone
working in these files. Part of this issue is moving the rule somewhere live.

Sketch

STAGES drops both columns -> stage name maps to (analysis class, batch
builder). run_stage becomes:

stage_class, batch = build_stage(...)
runner = BatchRunner(station, ExptClass=EncSpec, ExptProgram=batch.program, ...)
return stage_class.from_batch(runner.execute(batch.configs, ...), station=station)

This removes the need for all three symptom patches above.

Loose ends

  • run_stage's off-prod branch (mbr_campaign.py:362-364) also instantiates
    owner(...) per config; it becomes EncSpec too.
  • run_stage's return type changes from a raw aggregate to a stage class.
    Callers that already do .analyze() on the result keep working.
  • _from_expts uses cls.__new__(cls) (floquet_dark_mode_readout.py:66) to
    skip the acquisition __init__ it should not be inheriting. That can go once
    aggregates are no longer acquisition classes.
  • HDF5 filenames change, because they are built from the class name. Handled
    separately as part of the naming pass; not in scope here.

Blocks the jonginn -> main merge.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions