Skip to content

Conversation

@HGSilveri
Copy link
Collaborator

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

@googlebot googlebot added the cla: yes Makes googlebot stop complaining. label Jul 21, 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`.
@balopat
Copy link
Contributor

balopat commented Jul 23, 2020

Sorry for the wait on this @HGSilveri - I was distracted in the last two days with an event. Will be back to this in the coming days.

@HGSilveri
Copy link
Collaborator Author

No worries @balopat , thanks for all your help! By the way, I believe that the failed CI check is unrelated to this PR.

@balopat balopat self-assigned this Jul 27, 2020
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 is looking good, two small comments

(cirq.ops.HPowGate, cirq.ops.CNotPowGate, cirq.ops.CZPowGate,
cirq.ops.CCZPowGate, cirq.ops.CCXPowGate)):
expo = op.gate.exponent
valid_op = np.isclose(expo, np.around(expo, decimals=0))
Copy link
Contributor

Choose a reason for hiding this comment

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

How would this work with parametrized gates? The type of exponent is Union[float, sympy.Basic] - it could be parametrized. I think you should make sure you're handling that here and cover that flow with tests.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You're right, I made the appropriate changes.

@HGSilveri HGSilveri requested a review from balopat July 28, 2020 09:26
(cirq.ops.HPowGate, cirq.ops.CNotPowGate, cirq.ops.CZPowGate,
cirq.ops.CCZPowGate, cirq.ops.CCXPowGate)):
expo = op.gate.exponent
if not isinstance(expo, sympy.Basic):
Copy link
Contributor

@balopat balopat Jul 28, 2020

Choose a reason for hiding this comment

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

  1. Use cirq.is_parametrized(op) - that should take care of other parameters as well.
  2. Is the Pasqal device going to handle non-integer exponent and/or parametrized gates from PhasedXPowGate, XPowGate, YPowGate, ZPowGate? If not, then maybe we should just bring up the parametrization check further up, to the beginning in this method.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

  1. Ok, will do.
  2. Yes, we'll handle non-integer exponents for these. As for the parametrization, we won't necessarily handle that, but we support the parameter substitution for actual values in PasqalSampler.run_sweep, so we have to allow those here.

@HGSilveri HGSilveri requested a review from balopat July 29, 2020 08:02
@balopat balopat added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Jul 29, 2020
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Jul 29, 2020
@CirqBot CirqBot merged commit b21baea into quantumlib:master Jul 29, 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 29, 2020
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 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.

4 participants