Fix qid shape validation for zero-repetition measurements - #8318
Open
Prahalad-ship-it wants to merge 2 commits into
Open
Fix qid shape validation for zero-repetition measurements#8318Prahalad-ship-it wants to merge 2 commits into
Prahalad-ship-it wants to merge 2 commits into
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes a correctness issue in the zero-repetition measurement path of simulator.py.
When a circuit is sampled with repetitions == 0, repeated measurement keys were previously validated using only the number of qubits involved in each measurement operation. This was insufficient for Cirq's qudit model because two measurements can involve the same number of qubits while having different qid shapes.
As a result, repeated measurement keys could be incorrectly accepted when their underlying qid shapes were incompatible.
This change makes the zero-repetition path validate the full qid shape using protocols.qid_shape(op), matching the validation behavior already used by the relevant sampler implementations.
Regression tests are included for both zero-repetition record-shape preservation and mismatched qid-shape rejection.
Problem
The affected code path handles measurement results when there are zero requested repetitions.
For normal repetitions, the measurement machinery already performs the necessary validation of repeated measurement keys. However, the repetitions == 0 path constructs the measurement records separately and had its own validation logic.
That validation checked:
len(op.qubits)
rather than the complete qid shape.
Checking only the number of qubits is not sufficient for circuits containing qudits.
For example, consider two measurement operations using the same key:
one measurement involving a qid with shape (2,)
another measurement involving a qid with shape (3,)
Both operations have one qubit/qid, so:
len(op.qubits) == 1
for both.
However, their qid shapes are different:
protocols.qid_shape(op_1) == (2,)
protocols.qid_shape(op_2) == (3,)
Therefore, treating them as compatible measurements is incorrect.
The measurement key identifies a single logical measurement record, so repeated uses of that key need to have compatible qid shapes.
Root Cause
The root cause was a mismatch between the validation performed by the zero-repetition simulator path and the validation performed elsewhere in Cirq.
The zero-repetition path effectively treated:
len(op.qubits)
as sufficient information to determine whether repeated measurements were structurally compatible.
That assumption works for ordinary qubit-only measurements, because qubits all have dimension 2. It does not generalize to Cirq's qudit model, where qids can have different dimensions.
The established validation in the sampler-related code already uses the qid shape itself, so the zero-repetition simulator path was inconsistent with the surrounding architecture.
Fix
The zero-repetition measurement construction in simulator.py now uses:
protocols.qid_shape(op)
when validating repeated measurement keys.
The validation therefore considers the actual shape of the qids rather than only the number of qids involved.
If the same measurement key is reused with an incompatible qid shape, the code now raises the expected ValueError, consistent with the behavior of the existing sampler implementations.
The existing zero-repetition record structure is otherwise preserved.
For repeated measurement keys, the record shape remains:
(0, num_instances, len(qid_shape))
This is important because the fix should not alter the structure of valid zero-repetition results; it should only ensure that invalid repeated-key configurations are rejected.
Why this fix is correct
This change is intentionally narrow.
The goal is not to redesign measurement handling or change the public API. Instead, it brings the zero-repetition path into alignment with the existing semantic rules for measurement keys.
A measurement key represents a single logical measurement result. If that key is used by operations with incompatible qid shapes, there is no single consistent result shape that can represent those measurements.
Using protocols.qid_shape(op) captures this information directly.
This also avoids introducing a separate interpretation of qid compatibility in the simulator.
The relevant behavior is therefore now consistent across:
the zero-repetition simulator path
sampler.py
zeros_sampler.py
Behavior before the fix
For compatible repeated measurements, the existing behavior is preserved.
For incompatible repeated measurements, the zero-repetition path could previously accept cases where the number of qids was the same but their qid shapes differed.
Conceptually:
Measurement A:
key = "m"
qid shape = (2,)
Measurement B:
key = "m"
qid shape = (3,)
Both contain one qid, so a check based only on:
len(op.qubits)
would consider them compatible.
That is incorrect.
Behavior after the fix
The same example is now rejected because:
(2,) != (3,)
when the full qid shape is compared.
The code raises ValueError, matching the existing validation semantics used elsewhere in Cirq.
Valid repeated-key measurements continue to produce the expected zero-length measurement records.
Regression Tests
Two areas of regression coverage were added/updated in simulator_test.py.
The test verifies that repeated measurement keys continue to produce correctly shaped records when:
repetitions == 0
This ensures that tightening the validation does not accidentally change the valid result structure.
The expected structure remains:
(0, num_instances, len(qid_shape))
2. Mismatched qid shapes
A regression test verifies that repeated measurement keys with incompatible qid shapes are rejected.
The test specifically covers the case that previously could pass the zero-repetition validation because the measurements had the same number of qids but different qid dimensions.
This ensures that the bug cannot silently return in the future.
Validation
I ran the smallest relevant targeted test suites for the affected behavior.
Simulator tests
pytest -q simulator_test.py -k 'zero_repetitions or no_repetitions'
Result:
2 passed, 24 deselected
Zero sampler tests
pytest -q zeros_sampler_test.py -k 'repeated_keys or run_sweep'
Result:
2 passed, 2 deselected
These tests cover both the modified simulator behavior and the existing related sampler behavior.
Test Environment Notes
Pytest reported two non-fatal environment/configuration warnings:
an unknown asyncio_default_fixture_loop_scope configuration option
a .pytest_cache permission warning on Windows
Neither warning affected the targeted test results.
The relevant tests completed successfully.
I have intentionally not claimed that the entire Cirq test suite passes, since the validation performed for this focused fix was limited to the directly relevant tests.
Compatibility
This change does not modify the public API.
It does not change:
measurement key semantics
valid measurement result formats
simulator APIs
sampler APIs
qid definitions
circuit construction APIs
It only makes the zero-repetition validation correctly enforce the same qid-shape consistency already expected by the rest of the measurement infrastructure.
Performance Impact
The performance impact should be negligible.
The additional qid-shape comparison occurs only in the repetitions == 0 path, which is already a specialized edge case.
The change does not introduce a new expensive simulation operation, matrix calculation, circuit traversal, or repeated state-vector computation.
It replaces the insufficient compatibility check with the appropriate qid-shape check while the measurement operations are already being inspected.
Therefore, this fix should not materially affect normal simulation or sampling workloads.
Scope
This PR intentionally focuses on the zero-repetition correctness issue.
It does not attempt to:
redesign the simulator measurement implementation
optimize general simulator performance
modify qsim behavior
change measurement-key semantics
refactor unrelated sampler code
address unrelated qid-shape issues
introduce a new measurement validation abstraction
Keeping the change focused reduces the risk of regressions and makes the behavior easier to review.
Why the existing architecture was followed
The implementation follows the validation approach already established in Cirq rather than introducing new semantics.
In particular, sampler.py and zeros_sampler.py already treat incompatible qid shapes under the same measurement key as invalid.
The zero-repetition simulator path should follow the same rule.
This makes the fix consistent with the existing architecture instead of creating special behavior for one simulator execution path.
Result
After this change:
valid zero-repetition measurements continue to work
zero-repetition measurement record shapes remain correct
repeated measurement keys with compatible qid shapes remain valid
repeated measurement keys with incompatible qid shapes are rejected
the simulator's zero-repetition behavior is consistent with the existing sampler validation
no public API changes are required
targeted regression tests pass
Follow-up
A broader simulator profiling and optimization pass could be useful separately, particularly for larger-circuit workloads. However, that is outside the scope of this correctness fix and is not necessary to resolve the issue addressed by this PR.