-
Notifications
You must be signed in to change notification settings - Fork 11
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
Add MultiCNOT gate. #120
base: main
Are you sure you want to change the base?
Add MultiCNOT gate. #120
Conversation
Hello and welcome @SolidTux Thank you very much for your interest in qoqo and for your contribution! Before we can proceed to review the pull request and merge it we want to point out that like qiskit and cirq we only merge contributions covered by a contributor license agreement (CLA). The details of the CLA can be found in the CONTRIBUTE.md and CLA.pdf files in the repository. Kind regards |
Did you get the mail? |
Codecov Report
@@ Coverage Diff @@
## main #120 +/- ##
==========================================
- Coverage 84.87% 84.04% -0.84%
==========================================
Files 33 33
Lines 4753 4800 +47
==========================================
Hits 4034 4034
- Misses 719 766 +47
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Yes we did get it. Sorry for the delayed response, it's the usual end of the year rush. The changes look good to me, however we aim to keep the unittest coverage of qoqo/roqoqo high so before merging we would want to have additional unittests for the new operation. If you want to implement them, they should mostly look like the unittests for the MolmerSorensenXX operation in roqoqo/tests/integration/operations/multi_qubit_gate_operations.rs. And if you have any questions about the tests feel free to ask here. Kind regards |
No worries!
I'll do that and add a commit. I just wanted to have some feedback on the interface first. |
Hello @SolidTux, |
No worries, let me know if you need anything from me! |
Hello @SolidTux , |
Sorry for the long delay! I will try to add the tests for the Python part. But currently I get linking errors from pyO3, I'll try to debug them first. |
Hello @SolidTux, |
Thanks to Kirsten I can now run the Python part. I added some tests to qoqo, let me know if you need anything else. |
This adds a NOT gate with multiple controls. I also added the corresponding things to the qasm backend.
A conversion to a circuit is only implemented for 2 (which is of course redundant as CNOT exists) and 3. Maybe a dedicated CCNOT gate would be better, but I was not sure how to make that fit into the trait structure.
In general it would be great to make writing custom gates outside of this crate (at least for Rust code) and have
Circuit
returnVec<Box<dyn Operate>>
or something similar instead ofVec<Operation>
. But I don't know how that works on the Python side (and also how interoperation between two Python libraries with Rust backends works).