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

[Pasqal updates] PasqalDevice class generalization #3141

Merged
merged 15 commits into from
Jul 20, 2020

Conversation

HGSilveri
Copy link
Collaborator

This the second child-PR of #3126 , following #3133 .

  • Generalizes PasqalDevice, removing ties to the physical placement of the qubits.
  • PasqalDevice now serves as the parent class of all future Pasqal devices, only enforcing constraints expected to exist on all future devices.
  • PasqalDevicecan also be used as the device for submission of explicitly unconstrained circuits to Pasqal's devices.

A new device class, PasqalVirtualDevice, will subclass the new PasqalDevice and recover the hardware constraints of the current PasqalDevice on a future PR.

@googlebot googlebot added the cla: yes Makes googlebot stop complaining. label Jul 16, 2020
@HGSilveri
Copy link
Collaborator Author

Hi @balopat ! Here is the second PR. Also, I was wondering: is it okay to do two PR's simultaneously if they are unrelated? I suppose it is not recommended, I was just wondering since it could help speed up the process.

@balopat
Copy link
Contributor

balopat commented Jul 17, 2020

Hi @balopat ! Here is the second PR. Also, I was wondering: is it okay to do two PR's simultaneously if they are unrelated? I suppose it is not recommended, I was just wondering since it could help speed up the process.

Absolutely, feel free to open multiple ones! I'll have a look at this one today!

Copy link
Contributor

@balopat balopat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, with some comments on documentation and error messages mostly.



@cirq.value.value_equality
class PasqalDevice(cirq.devices.Device):
"""A Pasqal Device with qubits placed on a 3D grid."""
"""A generic Pasqal device."""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit more documentation here would be very helpful (you can just rephrase a bunch of the text you had in the documentation) for users to understand the intent of this class: When is this useful? Why would someone want to use an unconstrained Pasqal device vs the 3D one?


for q in qubits:
if not isinstance(q, ThreeDGridQubit):
if not isinstance(q, self.supported_qubit_type):
raise TypeError('Unsupported qubit type: {!r}'.format(q))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make the error message more helpful, you could list the supported qubit types.


@property
def maximum_qubit_number(self):
return 100
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we are so in the NISQ era ;)

if not isinstance(qub, ThreeDGridQubit):
raise ValueError('{} is not a 3D grid qubit '
if not isinstance(qub, self.supported_qubit_type):
raise ValueError('{} is not a valid qubit '
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to init, please clarify what are the accepted types for this device.

@HGSilveri HGSilveri requested a review from balopat July 20, 2020 09:04
@balopat balopat added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Jul 20, 2020
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Jul 20, 2020
@CirqBot CirqBot merged commit 4f20b00 into quantumlib:master Jul 20, 2020
@CirqBot CirqBot removed automerge Tells CirqBot to sync and merge this PR. (If it's running.) front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. labels Jul 20, 2020
CirqBot pushed a commit that referenced this pull request Jul 22, 2020
This the fourth child-PR of #3126 , following #3141  and in parallel with #3160 .

`ThreeDGridQubit` is removed from the `pasqal` module, as all dependencies have been eliminated and it can be fully replaced by `ThreeDQubit`.
CirqBot pushed a commit that referenced this pull request Jul 29, 2020
This the third child-PR of #3126 , following #3141 .

- The accepted gate set is updated to:
  - Accept only specific multi-qubit gates
  - Accept the Hadamard gate
- The noise model is updated accordingly and is now device dependent, in preparation for the addition of new devices whose noise model will naturally differ from the standard
- A specific converter for non-native gate decomposition is adapted from the `neutral_atoms` module
CirqBot pushed a commit that referenced this pull request Jul 31, 2020
This is the fifth child-PR of #3126, following  #3160.

`PasqalVirtualDevice`, which enforces the constraints typically found in a physical device, is added to the module. These constraints were present in `PasqalDevice` before its generalisation in #3141 . They are now reincorporated here.

This is the last code PR of the Pasqal updates, after which all that is left is to incorporate the documentation.
tonybruguier pushed a commit to tonybruguier/Cirq that referenced this pull request Aug 23, 2020
This the second child-PR of quantumlib#3126 , following quantumlib#3133 .

 - Generalizes `PasqalDevice`, removing ties to the physical placement of the qubits.
 - `PasqalDevice` now serves as the parent class of all future Pasqal devices, only enforcing constraints expected to exist on all future devices.
 - `PasqalDevice`can also be used as the device for submission of explicitly unconstrained circuits to Pasqal's devices.

A new device class, `PasqalVirtualDevice`, will subclass the new `PasqalDevice` and recover the hardware constraints of the current `PasqalDevice` on a future PR.
tonybruguier pushed a commit to tonybruguier/Cirq that referenced this pull request Aug 23, 2020
This the fourth child-PR of quantumlib#3126 , following quantumlib#3141  and in parallel with quantumlib#3160 .

`ThreeDGridQubit` is removed from the `pasqal` module, as all dependencies have been eliminated and it can be fully replaced by `ThreeDQubit`.
tonybruguier pushed a commit to tonybruguier/Cirq that referenced this pull request Aug 23, 2020
This the third child-PR of quantumlib#3126 , following quantumlib#3141 .

- The accepted gate set is updated to:
  - Accept only specific multi-qubit gates
  - Accept the Hadamard gate
- The noise model is updated accordingly and is now device dependent, in preparation for the addition of new devices whose noise model will naturally differ from the standard
- A specific converter for non-native gate decomposition is adapted from the `neutral_atoms` module
tonybruguier pushed a commit to tonybruguier/Cirq that referenced this pull request Aug 23, 2020
This is the fifth child-PR of quantumlib#3126, following  quantumlib#3160.

`PasqalVirtualDevice`, which enforces the constraints typically found in a physical device, is added to the module. These constraints were present in `PasqalDevice` before its generalisation in quantumlib#3141 . They are now reincorporated here.

This is the last code PR of the Pasqal updates, after which all that is left is to incorporate the documentation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Makes googlebot stop complaining.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants