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.

Sign up for GitHub

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

Jump to bottom

Gateset: Invalid repr if the gateset is empty #5197

Closed
verult opened this issue Apr 5, 2022 · 1 comment · Fixed by #5203 or #5322
Closed

Gateset: Invalid repr if the gateset is empty #5197

verult opened this issue Apr 5, 2022 · 1 comment · Fixed by #5203 or #5322
Assignees
@verult
Labels
area/gatesets kind/bug-report Something doesn't seem to work. triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on

Comments

@verult
Copy link
Collaborator

verult commented Apr 5, 2022 •
edited
Loading

Description of the issue
If a Gateset doesn't contain any gates, the repr contains an extra comma:

Cirq/cirq-core/cirq/ops/gateset.py

Lines 408 to 411 in e31b48d

name_str = f'name = "{self.name}", ' if self.name is not None else ''
return (
f'cirq.Gateset('
f'{self._gates_repr_str}, '

@tanujkhattar Should we fix the repr or make empty Gatesets invalid?

Cirq version
0.14
@verult verult added triage/discuss Needs decision / discussion, bring these up during Cirq Cynque kind/bug-report Something doesn't seem to work. time/before-1.0 labels Apr 5, 2022
@verult verult mentioned this issue Apr 6, 2022
Merged
@MichaelBroughton MichaelBroughton added triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on and removed triage/discuss Needs decision / discussion, bring these up during Cirq Cynque labels Apr 6, 2022
@tanujkhattar tanujkhattar added triage/discuss Needs decision / discussion, bring these up during Cirq Cynque and removed triage/discuss Needs decision / discussion, bring these up during Cirq Cynque labels Apr 6, 2022
@tanujkhattar
Copy link
Collaborator

We should fix this.

@verult verult mentioned this issue May 2, 2022
Merged
CirqBot pushed a commit that referenced this issue May 2, 2022
@verult
Gateset: fix invalid repr when gateset is empty (#5322)
c0c0e5f 
Fixes #5197

@tanujkhattar
CirqBot pushed a commit that referenced this issue May 9, 2022
@verult
cirq_google.GridDevice, minus gateset and gate durations (#5203)
4531a10 
Part of #5050

* Implemented qubit set, qubit pairs, and op validation. Gateset and gate duration will be done in a follow-up.
* Added a `device_specification_validation` module shell, to be used by the QCS server side to validate the DeviceSpecification proto against things like: 
  * Qubit self-loops in qubit pairs
  * Qubit pairs contain qubits not in the valid qubit set.
  * Gate durations contains a gate which is missing in the gateset.

`__str__()` and `_repr_pretty_` are nearly all copied from `SerializableDevice`.

Part of #5050

Also fixes #5197

**Question:** Does this definition of device equality make sense, or should it contain device name as well? Implementing equality is convenient for json and repr tests but not sure if it make sense in general.

@dstrain115 @MichaelBroughton
rht pushed a commit to rht/Cirq that referenced this issue May 1, 2023
@verult @rht
Gateset: fix invalid repr when gateset is empty (quantumlib#5322)
dad1ecc 
Fixes quantumlib#5197

@tanujkhattar
rht pushed a commit to rht/Cirq that referenced this issue May 1, 2023
@verult @rht
cirq_google.GridDevice, minus gateset and gate durations (quantumlib#…
448ca61 
…5203)

Part of quantumlib#5050

* Implemented qubit set, qubit pairs, and op validation. Gateset and gate duration will be done in a follow-up.
* Added a `device_specification_validation` module shell, to be used by the QCS server side to validate the DeviceSpecification proto against things like: 
  * Qubit self-loops in qubit pairs
  * Qubit pairs contain qubits not in the valid qubit set.
  * Gate durations contains a gate which is missing in the gateset.

`__str__()` and `_repr_pretty_` are nearly all copied from `SerializableDevice`.

Part of quantumlib#5050

Also fixes quantumlib#5197

**Question:** Does this definition of device equality make sense, or should it contain device name as well? Implementing equality is convenient for json and repr tests but not sure if it make sense in general.

@dstrain115 @MichaelBroughton
harry-phasecraft pushed a commit to PhaseCraft/Cirq that referenced this issue Oct 31, 2024
@verult
Gateset: fix invalid repr when gateset is empty (quantumlib#5322)
5132322 
Fixes quantumlib#5197

@tanujkhattar
harry-phasecraft pushed a commit to PhaseCraft/Cirq that referenced this issue Oct 31, 2024
@verult
cirq_google.GridDevice, minus gateset and gate durations (quantumlib#…
d630a83 
…5203)

Part of quantumlib#5050

* Implemented qubit set, qubit pairs, and op validation. Gateset and gate duration will be done in a follow-up.
* Added a `device_specification_validation` module shell, to be used by the QCS server side to validate the DeviceSpecification proto against things like: 
  * Qubit self-loops in qubit pairs
  * Qubit pairs contain qubits not in the valid qubit set.
  * Gate durations contains a gate which is missing in the gateset.

`__str__()` and `_repr_pretty_` are nearly all copied from `SerializableDevice`.

Part of quantumlib#5050

Also fixes quantumlib#5197

**Question:** Does this definition of device equality make sense, or should it contain device name as well? Implementing equality is convenient for json and repr tests but not sure if it make sense in general.

@dstrain115 @MichaelBroughton
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@verult verult

Labels
area/gatesets kind/bug-report Something doesn't seem to work. triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on
Projects
None yet
Milestone
No milestone
3 participants
@verult @MichaelBroughton @tanujkhattar