-
-
Notifications
You must be signed in to change notification settings - Fork 162
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
Upgrade Qiskit to 1.0.2 #2269
Upgrade Qiskit to 1.0.2 #2269
Conversation
This morning I took a look at the failing job. The Python 3.9 failures all relate to a dependency conflict involving Sphinx, the Sphinx theme, and nbsphinx, which is unrelated to this PR. Later this week I can look into why the docs are failing. I did not update the examples that use |
Also, the 3.11 failure seems to be an issue with Codecov: |
This was resolved by #2271. If you rebase main into your branch, and restart the test build, it should be fixed. |
The docs build is failing, as you suspected, because the qiskit notebooks from the examples haven't been updated. |
…2204/update_qiskit_to_1.0.2
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2269 +/- ##
==========================================
+ Coverage 98.29% 98.32% +0.03%
==========================================
Files 90 91 +1
Lines 4274 4308 +34
==========================================
+ Hits 4201 4236 +35
+ Misses 73 72 -1 ☔ View full report in Codecov by Sentry. |
|
Thanks, @andre-a-alves.
They are technically related to this PR in the sense that the conflicts are caused by the new dependencies introduced in this PR. In fact a clean build from the main branch on Python 3.9 is still working. I am not sure why in this PR it's trying to install myst-parser 1.0.0 on Python 3.9 🤷♂️ I pushed a commit to pin myst-parser to >= 2.0.0, let's see if it works. Also note the plan for Mitiq to drop support for Python 3.9 soon, see #2215. |
I did not realize. If it doesn't pass with the myst-parser pin, this evening I'll look for a combination of dependencies that work. |
# Modification made by: André Alves on 07 Apr 2024 | ||
# | ||
# For the original code and license, see | ||
# https://github.com/Qiskit/qiskit/blob/stable/1.0/qiskit/transpiler/passes/layout/apply_layout.py |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
License-wise, it's all good. My concern about having this file in mitiq is that it's going to be hard to keep it synced with the upstream qiskit file. Can't we just use the imported functions as they are, and wrap them where needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea is not to keep them in sync. I used a lot of the original ApplyLayout
, but I made substantive changes that are different from the original. The original class applies a QuantumRegister
with the name q
. We initialize the class with our own registers, and we use their names. From what I understand, maintaining the names is one of the purposes of the _transform
function. But with probably half the class identical to the original, I thought it appropriate to include the license statement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀
Since it's quite a big change, let's wait for a second review before merging.
Nice work @andre-a-alves!! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work Andre! Thanks for the help on this 💪🏻.
I've left a few small comments, but all the major stuff is there. Qiskit users will be very grateful!
All comments addressed in latest commit. Good catches! Thank you for the review! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* updates dev_requirements for Qiskit 1.0.2 * updates qiskit_utils to not use execute * updates conversions to use qasm2.dumps * updates I gate * adds TransformationPass to _transform_registers * updates test * updates zne tests * fixes ddd tests * fixes pec tests * fixes zne scaling tests * adds tests for transpiler * updates aer version * updates docs examples * slightly refactors qiskit transpilations * updates mitiq paper codeblock * pin myst-parser dependency * fixes docstring and adds tests for multi-register circuit * applies suggestions from comments --------- Co-authored-by: Alessandro Cosentino <alessandro@unitary.fund>
* updates dev_requirements for Qiskit 1.0.2 * updates qiskit_utils to not use execute * updates conversions to use qasm2.dumps * updates I gate * adds TransformationPass to _transform_registers * updates test * updates zne tests * fixes ddd tests * fixes pec tests * fixes zne scaling tests * adds tests for transpiler * updates aer version * updates docs examples * slightly refactors qiskit transpilations * updates mitiq paper codeblock * pin myst-parser dependency * fixes docstring and adds tests for multi-register circuit * applies suggestions from comments --------- Co-authored-by: Alessandro Cosentino <alessandro@unitary.fund>
Description
Closes #2208 #2204 #1422
Upgrades Qiskit to 1.0.2 and introduces a
TransformationPass
for transformingQuantumRegister
s.Out of ScopeDocs have not been updated to Qiskit 1.0. Some examples from the docs useqiskit.execute()
and remain incompatible with Qiskit 1.0.License
Before opening the PR, please ensure you have completed the following where appropriate.