Repository navigation
fix: Record measurements into named classical registers - #411
Conversation
Introduce ClassicalRegister and the register list on Circuit, with measurement columns derived from which register elements hold a measurement source. The context gets a scoped RegisterTable and every bit declaration creates a register. Nothing routes measurements through the registers yet: the old flat-index entry point survives as Circuit.add_measure_by_index, implemented on the anonymous register, and target_classical_indices is derived from it. Both are temporary and go away when measurements are recorded into declared registers. No behaviour change. The only visible difference is that Circuit.measured_qubits is now in classical index order instead of registration order, which is the order the simulator already used. Test preparation for the follow-up: MCM tests measure into one declared register instead of relying on the measure-all fallback.
| (because they had a classical destination but nothing forced them to be | ||
| applied) never affected control flow, so they are plain measurements of | ||
| the final state. Once execution has branched nothing is ever pending (see | ||
| ``_apply_pending_mcm_targets``). |
There was a problem hiding this comment.
I keep these docstring to give context about where and when these private functions are called. I feel they can be tricky to understand.
| @@ -2613,6 +2613,24 @@ def test_simple_conditional_feedforward(self, simulator): | |||
| assert set(counter.keys()) == {"00", "11"} | |||
| assert 0.4 < counter["00"] / 1000 < 0.6 | |||
|
|
|||
There was a problem hiding this comment.
In this test file, many tests have measurements become fully specified. The partial measurements is covered in test_scalar_bit_mcm_reports_only_the_measured_qubit.
Also, storing measurements into multiple registers becomes not allowed until the output is supported (we have PR opened: #399). Users must store measurement into a single register for now. I have changed few tests so that there are only one register for measurements.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #411 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 49 49
Lines 5050 5027 -23
Branches 590 579 -11
=========================================
- Hits 5050 5027 -23 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| Temporary entry point for callers that still address measurement columns by | ||
| a program-wide classical index. It is removed once measurements are routed | ||
| through declared registers via ``add_measure``. | ||
| def validate_single_measured_register(self) -> None: |
There was a problem hiding this comment.
This is a temporary validation and it will be removed once output is supported.
There was a problem hiding this comment.
To raise awareness: AutoQASM declares a new bit for every measure() call, so this check rejects most AutoQASM programs, including ones without mid-circuit measurements:
@aq.main(num_qubits=2)
def bell():
h(0)
cnot(0, 1)
measure(0)
measure(1)
LocalSimulator().run(bell, shots=100)Generated OpenQASM:
qubit[2] __qubits__;
h __qubits__[0];
cnot __qubits__[0], __qubits__[1];
bit __bit_0__;
__bit_0__ = measure __qubits__[0];
bit __bit_1__;
__bit_1__ = measure __qubits__[1];- Released simulator:
{'00': 51, '11': 49} - This PR:
ValueError: ... recorded into 2:bit_0,bit_1. Declare one bit register and measure into it.
AutoQASM users can't follow that advice because they don't write the declarations.
There was a problem hiding this comment.
also just for awareness - I have this PR stacked on top of Cody's output PR which makes everything work for AutoQASM programs: #401
We don't need to verify it now, but if/when we do rework output variable support, we should just ensure that PR also gets incorporated appropriately (and, ultimately, that amazon-braket/autoqasm#119 will pass CI once the output-supporting LocalSimulator is released)
The argument holds the destination index within the register for each measured qubit, so name it for that.
c216ab6 to
8442ffd
Compare
Measurement columns are measurement slots in the register model, so use that term in comments and docstrings.
8442ffd to
3c5a119
Compare
zip() silently dropped qubits when register_indices did not have one entry per measured qubit. Raise a ValueError before anything is bound.
3c5a119 to
36e3f8a
Compare
The register model does not change MCM semantics, so restore the original scalar-bit MCM programs. The rewrites they need move to the change that records measurements into declared registers.
36e3f8a to
40c48cf
Compare
A declaration whose initializer depends on a mid-circuit measurement was replayed in full for every path, so a bit declaration created one register per path. Declare the variable once and evaluate and assign the initializer per path. The initializer is evaluated before the declaration so a shadowed name still reads the outer variable.
071ae91 to
c681171
Compare
Declaring bit registers inside a for loop is not a supported pattern, so the tests should not depend on it. Coverage is unchanged.
Route every measurement with a classical destination through the register declared for it, so a measurement is recorded as "qubit q into element e of register r" instead of a program-wide classical index. The interpreter carries register identity from its scoped register table down to the circuit, and the simulator reports one column per measured register element in declaration then element order. Remove the temporary flat-index entry point (add_measure_by_index and the derived target_classical_indices) along with the three pending measurement flush paths it served; a single _apply_pending_mcm_targets applies all pending measurements together. Behaviour changes: - Distinct bit registers no longer alias each other's elements. - A scalar bit destination is one element and one column. - A shadowing bit declaration gets its own register. - Applying one pending measurement applies all of them, so a later pending measurement is not dropped from the results. - Measurements into a register are reported even after branching. - A classical write to a measured bit releases the measurement, so the element is no longer a column. - Results can only be reported for a single measured register until per-register results are available through the output field.
Follow the add_measure rename through the measurement destination plumbing: _resolve_destination, pending entries, _branch_measurement, and _release_measurements.
Use measurement slot in comments and docstrings for the register elements reported in measurements, and name the overlay loop variables after the slots they enumerate.
Each measured qubit pairs with exactly one register index and one bound slot. Make a length mismatch an error instead of silently dropping qubits.
Declaring bit registers inside a for loop is not a supported pattern. Reach the same out-of-scope paths with a subroutine whose local bit shadows the measurement destination.
c681171 to
985f866
Compare
Route every measurement with a classical destination through the register declared for it, so a measurement is recorded as "qubit q into element e of register r" instead of a program-wide classical index. The interpreter carries register identity from its scoped register table down to the circuit, and the simulator reports one column per measured register element in declaration then element order. Remove the temporary flat-index entry point (add_measure_by_index and the derived target_classical_indices) along with the three pending measurement flush paths it served; a single _apply_pending_mcm_targets applies all pending measurements together. Behaviour changes: - Distinct bit registers no longer alias each other's elements. - A scalar bit destination is one element and one column. - A shadowing bit declaration gets its own register. - Applying one pending measurement applies all of them, so a later pending measurement is not dropped from the results. - Measurements into a register are reported even after branching. - A classical write to a measured bit releases the measurement, so the element is no longer a column. - Results can only be reported for a single measured register until per-register results are available through the output field.
Follow the add_measure rename through the measurement destination plumbing: _resolve_destination, pending entries, _branch_measurement, and _release_measurements.
Use measurement slot in comments and docstrings for the register elements reported in measurements, and name the overlay loop variables after the slots they enumerate.
Each measured qubit pairs with exactly one register index and one bound slot. Make a length mismatch an error instead of silently dropping qubits.
Declaring bit registers inside a for loop is not a supported pattern. Reach the same out-of-scope paths with a subroutine whose local bit shadows the measurement destination.
985f866 to
8b239ef
Compare
nilsquet
left a comment
nilsquet
left a comment
There was a problem hiding this comment.
I added two comments inline and have some further nits:
- The description still mentions
_release_measurements, which was removed in1fa35ee. Circuit.clear_measurement(circuit.pyL175) andClassicalRegister.clear(circuit.pyL57) are now unused.- The description says a condition on a measured bit reads as 0 with shots = 0. That's only true inside the interpreter;
run_openqasm(shots=0)with a measurement still fails. program_context.pyL947–948: "For interpreter that supports multiple classical registers, the name of the register is another arg." is unclear. Suggest: "Contexts that support multiple classical registers receive the destination variable as theclassical_destinationkeyword argument."program_context.pyL1271: thedeclare_variabledocstring line is 116 characters, over the 100-character limit.
| # A deferred measurement may be flushed after its destination went out of | ||
| # scope (or was shadowed); the outcome is still recorded for ``measurements`` | ||
| # but there is no program variable left to update. | ||
| in_scope = self.register_table.get_register(register.name) is register |
There was a problem hiding this comment.
This crashes on a program that works on main:
def flip(qubit r) { bit b; x r; }
qubit[2] q;
bit b;
h q[0];
b = measure q[0];
flip(q[0]);
if (b) { x q[1]; }AttributeError: 'NoneType' object has no attribute 'value'
The x r inside flip forces the deferred measurement. At that point the name b points to the local b, so in_scope is False and the outer b is never set. After flip returns, if (b) reads None.
When the local is not a bit, there is no crash. The program returns wrong counts instead:
def flip(qubit r) { int c; x r; }
qubit[2] q;
bit[2] c;
h q[0];
c[0] = measure q[0];
flip(q[0]);
if (c[0]) { x q[1]; }
c[1] = measure q[1];Expected {00, 11}, but this PR returns {00, 10}: the outer c is never updated, so the if is never taken. Main crashes on this one, so it's not a regression from a working program, but a wrong result is harder to notice than a crash. _apply_pending_without_shots has the same name-based scope check.
The measurement already carries its register, so could we store the per-path value by register instead of looking it up by name? That would fix both cases and the shots = 0 path. test_shadowed_deferred_measurement_with_shots doesn't catch this because it never reads b after the call.
There was a problem hiding this comment.
Deferring the fix to another PR.
| updates (e.g., ``arr[0] = 5``) are handled by reading the current | ||
| value from the path, applying the index update, and writing back. | ||
| """ | ||
| self._flush_pending_mcm_for_variable(get_identifier_name(variable)) |
There was a problem hiding this comment.
This makes any write to a bit with a deferred measurement branch the simulation, even when nothing reads the result:
| Program (1000 shots) | main | PR |
|---|---|---|
qubit[12] q; bit[12] b; h q; b = measure q; b[0] = 0; |
0.23s | 22.6s |
qubit[10] q; bit[11] b; h q; b[0:9] = measure q; b[10] = 1; |
0.01s | 2.8s |
A write doesn't depend on the measured value. It replaces it, so it shouldn't need branching. Could we keep the measurement deferred and just mark the overwritten element so the flush skips writing it back? The results would still report the measured value.
There was a problem hiding this comment.
I tried yet another solution for constant overwrite. Instead of force flushing it, as you suggested, I mark a pending mcm as overwritten when an overwrite happens. The overwritten mark prevent a pending mcm writing value into a bit.
| Temporary entry point for callers that still address measurement columns by | ||
| a program-wide classical index. It is removed once measurements are routed | ||
| through declared registers via ``add_measure``. | ||
| def validate_single_measured_register(self) -> None: |
There was a problem hiding this comment.
To raise awareness: AutoQASM declares a new bit for every measure() call, so this check rejects most AutoQASM programs, including ones without mid-circuit measurements:
@aq.main(num_qubits=2)
def bell():
h(0)
cnot(0, 1)
measure(0)
measure(1)
LocalSimulator().run(bell, shots=100)Generated OpenQASM:
qubit[2] __qubits__;
h __qubits__[0];
cnot __qubits__[0], __qubits__[1];
bit __bit_0__;
__bit_0__ = measure __qubits__[0];
bit __bit_1__;
__bit_1__ = measure __qubits__[1];- Released simulator:
{'00': 51, '11': 49} - This PR:
ValueError: ... recorded into 2:bit_0,bit_1. Declare one bit register and measure into it.
AutoQASM users can't follow that advice because they don't write the declarations.
| program-wide classical index. It is removed once measurements are routed | ||
| through declared registers via ``add_measure``. | ||
| def validate_single_measured_register(self) -> None: | ||
| """Reject programs whose measurements span more than one register. |
There was a problem hiding this comment.
Again, for awareness: Whether this check fires depends on which branches run, so the same program can pass or fail from run to run:
qubit[2] q;
bit b;
bit r;
ry(0.3) q[0];
b = measure q[0];
if (b) { r = measure q[1]; }With 10 shots, 16 of 20 seeds return results and 4 raise ValueError. r only counts as measured when at least one shot takes the branch.
There was a problem hiding this comment.
Deferring the fix to the output PRs.
3 participants
Routes every measurement with a classical destination into the register declared for it, so a measurement is recorded as "qubit q into element e of register r" instead of a program-wide classical index. Removes the temporary flat-index API from #410 and consolidates the pending-measurement flush paths into one.
Stack: #410 (register model) → #411 (route measurements). Based on #410.
Background
#410 added
ClassicalRegister, the scopedRegisterTable, and a temporaryadd_measure_by_indexthat still wrote every measurement into the anonymous register. This PR connects the interpreter's register identity to the circuit, which fixes register aliasing and scope shadowing for measurement destinations.It also fixes an inconsistency between the two simulation flows. On
main,_branch_measurementonly records a measurement in the circuit when the destination is indexed. A scalar destination (b = measure q[0]) in a branched program never reaches the circuit, so_run_branchedsees no measurements and reports every qubit. The same program without branching reports only q[0].Results are still reported as one flat
measurementsarray. Until per-register results are available through theoutputfield, a program that measures into more than one register is rejected with a clear error instead of returning bits that cannot be attributed to a register.Behavior changes
ValueError. Measure into one register instead, for examplebit[2] b;withb[0]andb[1]. Declaring bits that are never measured into is still allowed.b = measure q[0]) reported every qubit.int x; x = measure q;, raisesTypeError.measurements.b[0] = measure q[0]; b[0] = 0; if (b[0]) { ... }the measurement could replace the written 0, so the branch was taken.measurementsstill reports the measured outcome, the same asmain.run_openqasm(shots=0)with a measurement still fails.IndexErrorinstead of writing to hidden indices: an index outside the register, or a register smaller than the number of measured qubits.More details about the changes
openqasm/program_context.pyadd_measureresolves the destination to(register, register_indices)once through_resolve_destination. A measurement with no destination goes into the anonymous register and never branches.(target, register, register_indices, overwritten). Matching uses the register handle, so a shadowing declaration never flushes the outer variable's measurement.NoneinRegisterTable, so it hides an outer bit of the same name._apply_pending_mcm_targetsreplaces_maybe_transition_to_branchedand the per-variable and per-qubit flush paths._branch_measurementalways records the measurement in the circuit._record_measurement_outcomesreplaces_update_identifier_targetand_update_indexed_target. A scalarbitis a size 1 register.update_valuemarks classically written elements on pending measurement entries so that applying the measurement later does not overwrite the program's value. A write never forces a flush.zip(..., strict=True), so a qubit and register index count mismatch raises instead of dropping qubits.get_valueandget_value_by_identifierno longer wrap raw path values into literals.openqasm/circuit.pyadd_measure_by_index,target_classical_indices,Circuit.clear_measurement, andClassicalRegister.clear.ClassicalRegister.measured,measured_registers, andvalidate_single_measured_register.openqasm/simulation_path.pymcm_outcomesis keyed by(register, register_index)instead of a flat classical index, and exposed as a property.simulator.pyvalidate_single_measured_registerand buildmeasuredQubitsfrommeasurement_slots.