Allow measurement keys with differing qubits in OpenQASM #6803
Conversation
- Previously, if there was a measurement with the same key but two differently sized registers, then the qasm output might select the wrong key and size the register incorrectly. - This PR examines all the measurements first, and selects the biggest one, so to correctly size the classical register. - Adds unit test t demonstrate. Fixes: quantumlib#6508
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6803 +/- ##
=======================================
Coverage 97.85% 97.85%
=======================================
Files 1084 1084
Lines 93731 93742 +11
=======================================
+ Hits 91722 91733 +11
Misses 2009 2009 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
| @@ -289,22 +289,25 @@ def output(text): | |||
| output(f'qubit[{len(self.qubits)}] q;\n') | |||
| # Classical registers | |||
| # Pick an id for the creg that will store each measurement | |||
| already_output_keys: Set[str] = set() | |||
| # cregs will store key -> (#qubits, comment) | |||
| cregs: Dict[str, tuple[int, str]] = {} | |||
There was a problem hiding this comment.
If you did this in the QasmOutput constructor and stored it in QasmArgs, you could pass it into KeyCondition.qasm, and fix #5691.
The reason it's required is that KeyCondition is triggered if meas[m_key] != 0. In OpenQasm2, the only logical comparator is equality, so != 0 can only be represented if the creg is a single bit (i.e. it would convert the condition to if m_key == 1). If the creg is multiple bits, then if m_key == 1 would be incorrect, and m_key != 0 is not representable. So that's why the existing behavior is to play it safe and just fail.
Going a little bit further, in OpenQasm3, it looks like m_key != 0 is representable, so checking QasmArgs.version in KeyCondition.qasm would allow supporting multi-bit classical controls if version == '3.0'. OpenQasm3 could also potentially support serializing additional types of logic in SympyConditions.
There was a problem hiding this comment.
It might also be useful to have this be a function on Circuit or part of the measurement_keys protocol. I'd imagine "get the shapes of all the measurement keys in the circuit" would be a common need.
There was a problem hiding this comment.
Changed to put into constructor, but I am not convinced that helps, since we don't pass QasmOutput during the qasm protocol. In any case, I will deal with classical controls in a follow-up PR.
Also, Circuit has enough cruft on it that I don't think it's a good idea to add more functions onto it, so I will leave it here.
| @@ -203,6 +203,7 @@ def __init__( | |||
| qubit_id_map=qubit_id_map, | |||
| meas_key_id_map=meas_key_id_map, | |||
| ) | |||
| self.cregs = self._generate_cregs() | |||
There was a problem hiding this comment.
Oh, I see. That makes more sense. I will look into this in a follow-up PR.
Co-authored-by: Pavol Juhas <[email protected]>
Co-authored-by: Pavol Juhas <[email protected]>
Co-authored-by: Pavol Juhas <[email protected]>
Fixes: #6508