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

Single classical bit conditioning breaks various functions #6475

Open
6 of 11 tasks
TharrmashasthaPV opened this issue May 28, 2021 · 3 comments
Open
6 of 11 tasks

Single classical bit conditioning breaks various functions #6475

TharrmashasthaPV opened this issue May 28, 2021 · 3 comments
Labels
bug Something isn't working

Comments

@TharrmashasthaPV
Copy link
Contributor

TharrmashasthaPV commented May 28, 2021

Information

  • Qiskit Terra version: 0.18.0.dev0+f6b2c43
  • Python version: 3.7

What is the current behavior?

The PR #6018 enables classical conditioning of gates on individual bits. However, as mentioned in that PR, this new feature breaks the following functions:

Suggested solutions

The fixes should be simple. Need to look for parts that have ClassicalRegister in condition in these functions and extend them also to cases when Clbit is in condition.

@mtreinish
Copy link
Member

I just added qpy serialization support to the list of things here. It was pointed out to me that trying to dump (and likely load too) a circuit with a single bit condition won't serialize correctly and error. The source is this line:

https://github.com/Qiskit/qiskit-terra/blob/main/qiskit/circuit/qpy_serialization.py#L699

because a Clbit object doesn't have a name. We might need a qpy format v2 to adjust the condition representation in the serialization format (https://github.com/Qiskit/qiskit-terra/blob/main/qiskit/circuit/qpy_serialization.py#L177-L179 ) to handle the lack of a register. Although, thinking out loud we may be able to hack it in by setting the name to be a the str(index) of the clbit in the circuit since that is not a valid register name.

mtreinish added a commit to mtreinish/qiskit-core that referenced this issue Jul 20, 2021
In Qiskit#6018 initial support for adding classical conditions on a single bit
instead of a register was added. However this wasn't accounted for in
qpy because it didn't exist when qpy was first written. This commit adds
support to qpy without a file format change so it can be backported and
used with already released versions of qpy without needing a new format
version. This is accomplished by exploiting the strict register naming
in qiskit. A register can't have a name outside of regex
"[a-z][a-zA-Z0-9_]*" which we leverage in the case of single clbit
conditions the "register" name in the output QPY data is set to
str(clbit_index) which isn't a valid name. Then on the loading side we
check for a valid name, if it's outside the allowed regex we treat the
condition as a single bit and the name is a str(index). The tradeoff here
is that for a QPY file generated with 0.18.1 (assuming this is backported
and included in 0.18.1) this qpy file can not be loaded with qiskit
0.18.0. But this is fine as we only guarantee compatibility in one
direction (loading qpy files generated with older with qiskit with a
newer version of qiskit).

Fixes partially Qiskit#6475
@jakelishman
Copy link
Member

As I understand it, QASM 2 doesn't support conditional operations on anything other than a complete register, so the circuit

import qiskit.circuit.qpy_serialization
from qiskit.circuit import QuantumRegister, QuantumCircuit, ClassicalRegister, Clbit
qr = QuantumRegister(2)
cr = ClassicalRegister(8)
qc = QuantumCircuit(qr, cr)
qc.measure(0, cr[0])
qc.measure(1, cr[1])
qc.x(0).c_if(cr[0], 1)
qc.x(1).c_if(cr, 3)

can't be translated into valid QASM 2. I had intended to go around this by emitting additional 1-bit classical registers and copying the data, but there's also no classical assignment operation, so that doesn't work either.

In theory it is possible to emit valid QASM if the only classical bits used aren't attached to registers, though - instead of binding them all into a single creg regless[...]; we separate them. I don't know if we want to put the effort into supporting this if we can't also support subscripts of ClassicalRegister?

I suspect the "best" solution to this within the framework of QASM 2 is just to emit QasmError in this case? It doesn't seem very satisfying, though.

@kdk kdk added this to the 0.19 milestone Jul 21, 2021
mergify bot added a commit that referenced this issue Jul 29, 2021
* Handle single bit conditions in QPY

In #6018 initial support for adding classical conditions on a single bit
instead of a register was added. However this wasn't accounted for in
qpy because it didn't exist when qpy was first written. This commit adds
support to qpy without a file format change so it can be backported and
used with already released versions of qpy without needing a new format
version. This is accomplished by exploiting the strict register naming
in qiskit. A register can't have a name outside of regex
"[a-z][a-zA-Z0-9_]*" which we leverage in the case of single clbit
conditions the "register" name in the output QPY data is set to
str(clbit_index) which isn't a valid name. Then on the loading side we
check for a valid name, if it's outside the allowed regex we treat the
condition as a single bit and the name is a str(index). The tradeoff here
is that for a QPY file generated with 0.18.1 (assuming this is backported
and included in 0.18.1) this qpy file can not be loaded with qiskit
0.18.0. But this is fine as we only guarantee compatibility in one
direction (loading qpy files generated with older with qiskit with a
newer version of qiskit).

Fixes partially #6475

* Add release note

* Prefix bit index with null character

This commit modifies the special string we use in the register name
field to be prefixed with a null character. This leaves open using the
string for other special cases and also returning a sane error if an
invalid QPY file was generated by something besides Qiskit.

* Update qiskit/circuit/qpy_serialization.py

Co-authored-by: Jake Lishman <jake@binhbar.com>

* Add note to release note about feature not being supported

* Add backwards compat test case too

Co-authored-by: Jake Lishman <jake@binhbar.com>
Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
mergify bot pushed a commit that referenced this issue Jul 29, 2021
* Handle single bit conditions in QPY

In #6018 initial support for adding classical conditions on a single bit
instead of a register was added. However this wasn't accounted for in
qpy because it didn't exist when qpy was first written. This commit adds
support to qpy without a file format change so it can be backported and
used with already released versions of qpy without needing a new format
version. This is accomplished by exploiting the strict register naming
in qiskit. A register can't have a name outside of regex
"[a-z][a-zA-Z0-9_]*" which we leverage in the case of single clbit
conditions the "register" name in the output QPY data is set to
str(clbit_index) which isn't a valid name. Then on the loading side we
check for a valid name, if it's outside the allowed regex we treat the
condition as a single bit and the name is a str(index). The tradeoff here
is that for a QPY file generated with 0.18.1 (assuming this is backported
and included in 0.18.1) this qpy file can not be loaded with qiskit
0.18.0. But this is fine as we only guarantee compatibility in one
direction (loading qpy files generated with older with qiskit with a
newer version of qiskit).

Fixes partially #6475

* Add release note

* Prefix bit index with null character

This commit modifies the special string we use in the register name
field to be prefixed with a null character. This leaves open using the
string for other special cases and also returning a sane error if an
invalid QPY file was generated by something besides Qiskit.

* Update qiskit/circuit/qpy_serialization.py

Co-authored-by: Jake Lishman <jake@binhbar.com>

* Add note to release note about feature not being supported

* Add backwards compat test case too

Co-authored-by: Jake Lishman <jake@binhbar.com>
Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit afb0c47)
mergify bot added a commit that referenced this issue Jul 29, 2021
* Handle single bit conditions in QPY

In #6018 initial support for adding classical conditions on a single bit
instead of a register was added. However this wasn't accounted for in
qpy because it didn't exist when qpy was first written. This commit adds
support to qpy without a file format change so it can be backported and
used with already released versions of qpy without needing a new format
version. This is accomplished by exploiting the strict register naming
in qiskit. A register can't have a name outside of regex
"[a-z][a-zA-Z0-9_]*" which we leverage in the case of single clbit
conditions the "register" name in the output QPY data is set to
str(clbit_index) which isn't a valid name. Then on the loading side we
check for a valid name, if it's outside the allowed regex we treat the
condition as a single bit and the name is a str(index). The tradeoff here
is that for a QPY file generated with 0.18.1 (assuming this is backported
and included in 0.18.1) this qpy file can not be loaded with qiskit
0.18.0. But this is fine as we only guarantee compatibility in one
direction (loading qpy files generated with older with qiskit with a
newer version of qiskit).

Fixes partially #6475

* Add release note

* Prefix bit index with null character

This commit modifies the special string we use in the register name
field to be prefixed with a null character. This leaves open using the
string for other special cases and also returning a sane error if an
invalid QPY file was generated by something besides Qiskit.

* Update qiskit/circuit/qpy_serialization.py

Co-authored-by: Jake Lishman <jake@binhbar.com>

* Add note to release note about feature not being supported

* Add backwards compat test case too

Co-authored-by: Jake Lishman <jake@binhbar.com>
Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit afb0c47)

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
@mtreinish mtreinish modified the milestones: 0.19, 0.20 Nov 17, 2021
@HuangJunye
Copy link
Collaborator

@TharrmashasthaPV we are planning to release 0.20 soon. Are you planning to fix the rest of issues listed here?

@mtreinish mtreinish modified the milestones: 0.20, 0.21 Mar 26, 2022
@kdk kdk removed this from the 0.21 milestone Jun 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants