Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect Classical Register Size in to_qasm with Inhomogeneous Measurements #6508

Open
p51lee opened this issue Mar 20, 2024 · 2 comments
Open
Labels
kind/bug-report Something doesn't seem to work. triage/accepted there is consensus amongst maintainers that this is a real bug or a reasonable feature to add triage/needs-more-evidence [feature requests] this seems plausible but maintainers are not convinced about the use cases yet

Comments

@p51lee
Copy link

p51lee commented Mar 20, 2024

Description of the issue

When a Circuit contains multiple measurements under the same key but with varying sizes, the to_qasm method improperly assigns the classical register size, resulting in a smaller than required register.
A potential solution includes �fixing this behavior directly or, throwing an exception for circuits with measurements of differing sizes being translated to OpenQASM, to prevent incorrect translations.

How to reproduce the issue

import cirq

qubits = cirq.LineQubit.range(2)
circuit = cirq.Circuit(
    cirq.measure(qubits[0], key='c'),
    cirq.measure(qubits, key='c'),
)
# No issue
# circuit = cirq.Circuit(
#    cirq.measure(qubits, key='c'),
#    cirq.measure(qubits[0], key='c'),
# )

print(circuit.to_qasm())

Result:

// Generated from Cirq v1.3.0

OPENQASM 2.0;
include "qelib1.inc";


// Qubits: [q(0), q(1)]
qreg q[2];
creg m_c[1];  // Incorrectly suggests a single-bit register, expected size is 2


measure q[0] -> m_c[0];

// Gate: cirq.MeasurementGate(2, cirq.MeasurementKey(name='c'), ())
measure q[0] -> m_c[0];
measure q[1] -> m_c[1]; 

Cirq version
1.3.0

@p51lee p51lee added the kind/bug-report Something doesn't seem to work. label Mar 20, 2024
@pavoljuhas pavoljuhas added triage/needs-more-evidence [feature requests] this seems plausible but maintainers are not convinced about the use cases yet triage/accepted there is consensus amongst maintainers that this is a real bug or a reasonable feature to add labels Mar 27, 2024
@verult
Copy link
Collaborator

verult commented Mar 27, 2024

Thank you for the bug report. From the Cirq sync: to help make a decision regarding whether a fix or an exception is more appropriate, @p51lee do you have concrete use cases in mind where two qubit registers with the same name and different lengths are measured with the same key?

In either case, we should also consider adding documentation about the expected behavior where appropriate.

@p51lee
Copy link
Author

p51lee commented Mar 28, 2024

Thank you for following up. The issue occurs even when using qubits with different names:

qubits_1 = [cirq.NamedQubit("p")]
qubits_2 = [cirq.NamedQubit("q"), cirq.NamedQubit("r")]
circuit = cirq.Circuit(
    cirq.measure(qubits_1, key='c'),
    cirq.measure(qubits_2, key='c'),
)
print(circuit.to_qasm())

For a use case, I considered circuits with classically conditioned gates. However, since Cirq doesn’t support conditional QASM output, an exception for measurements with different sizes but the same key seems appropriate. This would prevent confusion and incorrect QASM translations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug-report Something doesn't seem to work. triage/accepted there is consensus amongst maintainers that this is a real bug or a reasonable feature to add triage/needs-more-evidence [feature requests] this seems plausible but maintainers are not convinced about the use cases yet
Projects
None yet
Development

No branches or pull requests

3 participants