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

str(GridQubit) too similar to str(tuple) #2405

Closed
mpharrigan opened this issue Oct 25, 2019 · 11 comments · Fixed by #5343
Closed

str(GridQubit) too similar to str(tuple) #2405

mpharrigan opened this issue Oct 25, 2019 · 11 comments · Fixed by #5343
Assignees
Labels
area/circuits area/qubits area/visualization kind/feature-request Describes new functionality no QC knowledge needed Want to contribute to Cirq, but don't know quantum computing? This issue is for you. triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on

Comments

@mpharrigan
Copy link
Collaborator

Was just waylaid in debugging an issue because I thought I was dealing with two-tuples but it was actually a GridQubit. Their str representation is indistinguishable. I suggest adding a subtle hint that it's different.

The closest analogy I can think of is the difference between a python list and a numpy array

In[3]: l = [1, 2, 3]
In[4]: print(l)
[1, 2, 3]
In[5]: print(np.asarray(l))
[1 2 3]
@Strilanc
Copy link
Contributor

This can be a bit of a nasty change to make, because it affects circuit diagrams.

We may want to separate the circuit representation of a qubit from its __str__.

@mpharrigan
Copy link
Collaborator Author

Yes, I find it surprising that the circuit representation relies on __str__. Everything else (i.e. gates) uses _circuit_diagram_info_

@dstrain115 dstrain115 self-assigned this Nov 14, 2019
@dabacon
Copy link
Collaborator

dabacon commented Apr 15, 2020

One suggestion is q(1,2), which we could then LineQubit could be q(1) and NamedQubit could be q(foo). I think this is better than modifying just GridQubit because it is more universal.

@Strilanc
Copy link
Contributor

Agree that q(row, col) is superior to row_col for our purposes because it can be consistent across qubit types.

@dstrain115
Copy link
Collaborator

Are we okay with doing that change if it breaks circuit diagrams and other things that depend on str representations?

@mpharrigan
Copy link
Collaborator Author

My initial preference is for keeping circuit diagrams the same. Context is clear in circuit diagram anyways.

Changing the str for qubits might break things as well? I've seen str(q) used as measurement keys. This probably would still work in the majority of cases where str(q) is used to retrieve measurements by key, but wouldn't if people were re-constructing strings or re-processing saved results

@cduck
Copy link
Collaborator

cduck commented Apr 15, 2020

Qubits could have their own version of _circuit_diagram_info_ so the diagram isn't dependent on the string representation. This would allow a more clear str(q) without changing diagrams.

@balopat balopat added area/circuits area/qubits area/visualization kind/feature-request Describes new functionality status/needs-agreed-design We want to do this, but it needs an agreed upon design before implementation triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on labels Sep 17, 2020
@dstrain115 dstrain115 added the triage/discuss Needs decision / discussion, bring these up during Cirq Cynque label Mar 30, 2022
@dstrain115
Copy link
Collaborator

Resurfacing this as a discuss item, so we can figure out if this is really needed or not.

@maffoo
Copy link
Contributor

maffoo commented Mar 30, 2022

I like @dabacon's suggestion about q(1, 2) with support for line qubits and named qubits as well. Could even have a cirq.q function that can be called in each of these ways and is overloaded to produce the appropriate qubit type.

@MichaelBroughton MichaelBroughton added no QC knowledge needed Want to contribute to Cirq, but don't know quantum computing? This issue is for you. and removed triage/discuss Needs decision / discussion, bring these up during Cirq Cynque status/needs-agreed-design We want to do this, but it needs an agreed upon design before implementation labels Apr 6, 2022
@tanujkhattar
Copy link
Collaborator

From cirq sync:

We should do this!

@dabacon
Copy link
Collaborator

dabacon commented Apr 19, 2022

cirq.q added in #5181

dstrain115 added a commit to dstrain115/Cirq-1 that referenced this issue May 10, 2022
 - Changes grid qubit string representation from (x, y) to q(x, y)
 and line qubit from 1 to q1.
 - This will make the string representation more distinctive.
 - Added _circuit_diagram_info_ for Qid objects to make circuit diagrams
customizable for qubits.  This allows us to keep circuit diagrams
largely unchanged.
 - Other components such as pauli strings, that rely on qubit str
   representation for formatting are changed.

 This is a BREAKING CHANGE for anyone relying on string representation
 of qubits.

Fixes: quantumlib#2405
dstrain115 added a commit that referenced this issue May 16, 2022
* Change qubit str representation

 - Changes grid qubit string representation from (x, y) to q(x, y)
 and line qubit from 1 to q1.
 - This will make the string representation more distinctive.
 - Added _circuit_diagram_info_ for Qid objects to make circuit diagrams
customizable for qubits.  This allows us to keep circuit diagrams
largely unchanged.
 - Other components such as pauli strings, that rely on qubit str
   representation for formatting are changed.

 This is a BREAKING CHANGE for anyone relying on string representation
 of qubits.

Fixes: #2405

* Fix a bunch of tests.

* A few more errors.

* Switch to q(0) and fix a bunch of tests.

* Formatting and more tests fixed.

* Fix more tests.

* Fix two last tests.

* Fix bb84 example.

* Fix contrib and notebook tests.

* Update cirq-core/cirq/sim/simulator_test.py

Co-authored-by: Matthew Neeley <[email protected]>

* Update cirq-core/cirq/circuits/circuit.py

Co-authored-by: Matthew Neeley <[email protected]>

* Revert Grid Device to use circuit diagram names instead.

Co-authored-by: Matthew Neeley <[email protected]>
rht pushed a commit to rht/Cirq that referenced this issue May 1, 2023
* Change qubit str representation

 - Changes grid qubit string representation from (x, y) to q(x, y)
 and line qubit from 1 to q1.
 - This will make the string representation more distinctive.
 - Added _circuit_diagram_info_ for Qid objects to make circuit diagrams
customizable for qubits.  This allows us to keep circuit diagrams
largely unchanged.
 - Other components such as pauli strings, that rely on qubit str
   representation for formatting are changed.

 This is a BREAKING CHANGE for anyone relying on string representation
 of qubits.

Fixes: quantumlib#2405

* Fix a bunch of tests.

* A few more errors.

* Switch to q(0) and fix a bunch of tests.

* Formatting and more tests fixed.

* Fix more tests.

* Fix two last tests.

* Fix bb84 example.

* Fix contrib and notebook tests.

* Update cirq-core/cirq/sim/simulator_test.py

Co-authored-by: Matthew Neeley <[email protected]>

* Update cirq-core/cirq/circuits/circuit.py

Co-authored-by: Matthew Neeley <[email protected]>

* Revert Grid Device to use circuit diagram names instead.

Co-authored-by: Matthew Neeley <[email protected]>
harry-phasecraft pushed a commit to PhaseCraft/Cirq that referenced this issue Oct 31, 2024
* Change qubit str representation

 - Changes grid qubit string representation from (x, y) to q(x, y)
 and line qubit from 1 to q1.
 - This will make the string representation more distinctive.
 - Added _circuit_diagram_info_ for Qid objects to make circuit diagrams
customizable for qubits.  This allows us to keep circuit diagrams
largely unchanged.
 - Other components such as pauli strings, that rely on qubit str
   representation for formatting are changed.

 This is a BREAKING CHANGE for anyone relying on string representation
 of qubits.

Fixes: quantumlib#2405

* Fix a bunch of tests.

* A few more errors.

* Switch to q(0) and fix a bunch of tests.

* Formatting and more tests fixed.

* Fix more tests.

* Fix two last tests.

* Fix bb84 example.

* Fix contrib and notebook tests.

* Update cirq-core/cirq/sim/simulator_test.py

Co-authored-by: Matthew Neeley <[email protected]>

* Update cirq-core/cirq/circuits/circuit.py

Co-authored-by: Matthew Neeley <[email protected]>

* Revert Grid Device to use circuit diagram names instead.

Co-authored-by: Matthew Neeley <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/circuits area/qubits area/visualization kind/feature-request Describes new functionality no QC knowledge needed Want to contribute to Cirq, but don't know quantum computing? This issue is for you. triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants