Skip to content

refactor(gemini): make loss normalization an abstract backend classmethod, not a task override #889

Closed

Description

@weinbe58

Summary

Follow-up to #838. _normalize_matrix should not live on the simulator task. It should be an abstract classmethod on AbstractSimulatorBackend, implemented per backend, and it should take the device's readout capability as an explicit input.

Loss normalization sits at the intersection of two independent capabilities:

  1. How sophisticated the simulator is — does this backend model atom loss at all, and what sentinel does it use to report it?
  2. What the modeled machine can read out — does the device have loss-resolved readout, or is a lost atom indistinguishable from |1>?

Today both are hard-coded in the task layer, so neither is expressible.

Current state

_SimulatorTaskBase._normalize_matrix (_task_runtime.py:283) is a staticmethod parameterized by loss_replace / is_loss, and it is overridden — identically — in two task classes:

Both overrides do the same thing: is_loss = lambda v: v is None, loss_replace = True. That None sentinel is a property of exactly one backend — PPVMSimulatorBackend.sample maps ppvm.MeasurementResult.LOST to None (simulator_backend.py:419) — while TsimSimulatorBackend and _PyQrackSimulatorBackend return dtype=bool arrays that can never contain a loss value.

Concrete problems with that placement:

  • The task encodes a backend detail. A new backend with a different loss sentinel (an enum member, a masked array, a separate loss matrix, -1, NaN) requires editing every task subclass rather than the backend that produced it.
  • Duplicated override. Two byte-identical overrides exist purely to change two defaults, and any future task class must remember to repeat them.
  • The device capability is unexpressible. There is currently no loss_readout / supports_loss flag anywhere in the repo (grep finds none). "Gemini has no loss-resolved readout" is baked in as loss_replace=True at the task, so a machine or simulator target with loss readout has no way to surface loss to the user.
  • Silent data loss in the base default. The base defaults are is_loss = (v is None), loss_replace = None, which the docstring calls a "no-op". It isn't: np.asarray([[False, None]], dtype=object).astype(bool) yields [[False, False]], so any backend routed through the base path silently reports lost atoms as 0 — the opposite of the physically correct fallback (1).

Requested design

Add to AbstractSimulatorBackend:

  1. A capability declaration — e.g. models_loss: ClassVar[bool] or supports_loss_modeling() — stating whether this simulator can produce loss outcomes at all.
  2. An abstract classmethod that normalizes a raw sample matrix into the uniform output encoding, given the device's readout capability:
@overload
@classmethod
def normalize_matrix(
    cls, payload: Any, *, name: str, shots: int, loss_readout: Literal[False]
) -> list[list[bool]]: ...

@overload
@classmethod
def normalize_matrix(
    cls, payload: Any, *, name: str, shots: int, loss_readout: Literal[True]
) -> list[list[int]]: ...

@classmethod
@abc.abstractmethod
def normalize_matrix(
    cls,
    payload: Any,
    *,
    name: str,
    shots: int,
    loss_readout: bool,   # does the modeled device expose loss-resolved readout?
) -> list[list[bool]] | list[list[int]]: ...

Shared shape/shots validation stays in one place (a module-level helper or a non-abstract base implementation the subclass calls); only the sentinel decoding is per backend. _SimulatorTaskBase.run then calls self._backend.normalize_matrix(..., loss_readout=<device capability>) and drops the task-level overrides.

loss_readout is a property of the device being modeled, not of the backend, so it must be threaded from the task/device/ArchSpec side. Default it to False for the current Gemini targets to preserve today's behavior.

Normalization table

device has loss readout simulator models loss output
yes yes 0 / 1, and 2 for the lost state — list[list[int]]
yes no 0 / 1 only, no loss outcomes exist to encode — list[list[bool]]
no yes every loss outcome rewritten to 1, since lost atoms read as |1> on hardware — list[list[bool]]
no no 0 / 1 only — list[list[bool]]

Rationale for row 3: loss happens physically, and a machine without loss-resolved readout always reports a lost atom as 1. A loss-capable simulator must therefore be degraded to match the device it is standing in for — otherwise loss-aware and loss-blind backends produce results the user cannot compare.

Return type: bool stays the guarantee without loss readout

The three-valued encoding is confined to the loss_readout=True path. A device without loss-resolved readout must keep returning list[list[bool]] exactly as it does today — that is a hard backward-compatibility requirement, not a preference. Rows 2, 3 and 4 of the table therefore all produce bool, and only row 1 produces int with 2 reachable.

Sequence is covariant and bool is a subclass of int, so the shared result surface (SimulatorResult.measurements, Result, DetectorResult) can widen its annotation to Sequence[Sequence[int]] without invalidating any existing Sequence[Sequence[bool]] value. Existing callers keep receiving real bool objects at runtime, so if outcome:, outcome == 1, sum(row) and numpy round-trips are all unchanged.

Detectors and observables: the Device's call, not this issue's

run currently pushes measurements, detectors and observables through the same function, and a 2 cannot flow into m2d conversion or detector arithmetic. That policy — when and how loss is resolved before detector conversion — is left to the concrete Device implementation so it does not block this refactor. This issue only requires that detector/observable normalization keep its current strict bool behavior (shape and shots validation via the shared helper) and not silently absorb a loss sentinel.

Guard the inconsistent case

If a backend declares models_loss = False but emits a loss sentinel anyway, raise instead of coercing — that combination is a backend bug, and the current astype(bool) hides it.

Acceptance criteria

  • AbstractSimulatorBackend declares the loss capability and the abstract normalization classmethod; every concrete backend (Tsim, CliffT, PPVM, _PyQrack) implements it.
  • _normalize_matrix overrides are removed from GeminiLogicalSimulatorTask and PhysicalSimulatorTask; _SimulatorTaskBase only supplies the device's loss_readout capability.
  • Shape/shots validation is not duplicated per backend.
  • Behavior for existing Gemini targets is unchanged: PPVM loss still surfaces as 1, Tsim/CliffT/PyQrack results are unaffected, and every loss_readout=False path still returns list[list[bool]].
  • Detector/observable normalization stays strictly bool and raises on an unresolved loss sentinel rather than coercing it.
  • Tests cover all four rows of the table, including a loss-readout-enabled device producing 2, and the inconsistent-backend error path. The existing task-level tests
    (test_physical_simulator.py:132, :141)
    move to backend-level tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    category: featureCategory: new feature or feature request.hardware: GeminiHardware: Gemini machine support and related issues, bug reports, and feature requests.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions