Updating Pasqal classes #3126
Updating Pasqal classes #3126
Conversation
|
Hi @balopat, thank you very much for the remarks! You are absolutely right, these are all accidental and unrelated to our classes. I will clean up every change that didn't come from our side ASAP, thank you for your patience. |
|
Hi again @balopat, it should be okay now. I apologise for the trouble! |
There was a problem hiding this comment.
Thank you for the cleanups and explanations!
Now, in order to get this change in, the fastest way will be (though might be unintuitive) to break it up into smaller chunks. We still have 28 files changed here. If we could limit each "child-PR" to <10 or even below that, that would be great.
You do have distinct features that could serve as a natural cutting point:
- PasqalDevice was modified to be the parent class of all Pasqal devices. Also allows submission of nearly unconstrained circuits to Pasqal processors.
- PasqalVirtualDevice enforces the constraints of a near-term Pasqal QPU.
- ThreeDGridQubit was renamed ThreeDQubit (and TwoDQubit was created as a subclass).
- More flexibility in the accepted qubits (PasqalVirtualDevice can also work with GridQubit and LineQubit)
- More adequate accepted gate set, now consistent with the hardware
- docs
Maybe 1+2, 3+4, 5 and 6?
The reason this strategy will work faster is that most of the team members and experts have typically small amount of times throughout the day for reviews, scattered around their other work. If we can do the nitpicking in isolation and small batches, we'll get a higher quality code altogether and make partial progress faster.
I hope you don't mind the extra toil around that. Thank you!
|
Hi again @balopat, thank you for your time and help so far. Regarding your recommendation, I understand your point. However, these changes are very much intertwined, so it is easier said that done. Let me elaborate on what I think, realistically, can be done:
To summarise, I suggest we divide the PR into a big PR (points 1-4 or 1-5), followed by one or two smaller PR's (just 6 or 5 and 6). I understand this is not ideal, but it is really impossible to naturally dissociate the changes in the Qubit classes and the Device classes, since they influence one another. Plus, forcing this split would still entail a disappointingly small reduction in the number of files per PR. |
|
Hey @HGSilveri, Thanks for thinking this through with me! This sounds good, let's do 1-4, 5, 6 separately then. It is not only about the number of files. It is also about the number of concepts that change or are introduced in a PR. Last year when I developed the QASM Importer, I ran into the same issue with only 11 files that I broke out and got much faster reviews - well it still took some time, but at the end it was really easier on the reviewers (https://github.com/quantumlib/Cirq/pulls?q=is%3Apr+is%3Aclosed+author%3Abalopat+qasm+import). Thank you! |
|
As discussed above, the PR will be closed in favour of a split into smaller PR's. |
This the first child-PR of #3126 . - Introduces new Pasqal qubit classes: `ThreeDQubit`and `TwoDQubit` (a subclass of `ThreeDQubit`) - These new classes are intended to replace the original `ThreeDGridQubit`, which will be deleted on the next PR (where other classes' dependencies will be resolved). - In comparison with `ThreeDGridQubit`, these new classes allow for arbitrary placement of qubits in 3D or 2D space, instead of restricting them to a grid. This more closely resembles the capabilities of Pasqal devices.
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. - `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.
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
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.
This the first child-PR of quantumlib#3126 . - Introduces new Pasqal qubit classes: `ThreeDQubit`and `TwoDQubit` (a subclass of `ThreeDQubit`) - These new classes are intended to replace the original `ThreeDGridQubit`, which will be deleted on the next PR (where other classes' dependencies will be resolved). - In comparison with `ThreeDGridQubit`, these new classes allow for arbitrary placement of qubits in 3D or 2D space, instead of restricting them to a grid. This more closely resembles the capabilities of Pasqal devices.
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.
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`.
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
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.
Significant modifications to the first version of the
pasqalmodule. Most relevantly:PasqalDevicewas modified to be the parent class of all Pasqal devices. Also allows submission of nearly unconstrained circuits to Pasqal processors.PasqalVirtualDeviceenforces the constraints of a near-term Pasqal QPU.ThreeDGridQubitwas renamedThreeDQubit(andTwoDQubitwas created as a subclass).PasqalVirtualDevicecan also work withGridQubitandLineQubit)New additions: