-
Notifications
You must be signed in to change notification settings - Fork 59
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
Added V2 and ISA support #197
Draft
tnemoz
wants to merge
11
commits into
qiskit-community:main
Choose a base branch
from
LenaPer:main
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from 4 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
529c5da
Modified phase estimators to work with V2 primitives and ISA circuits
tnemoz e390037
Merge branch 'main' into main
woodsp-ibm da40425
Changed PassManager to more generic transpiler
tnemoz d83408c
Changed custom types to support Python 3.8
tnemoz 0e6e6f0
Added transpiler to .pylintdict
tnemoz 0e8ac74
Adapted Grover to V2 primitives
tnemoz 3d79bb5
Changed custom types using __future__ annotations
tnemoz eae6cff
Adapated VQD to V2 primitives
tnemoz df69ea7
Adapted tests for Grover
tnemoz 111188b
Fixed styling
tnemoz 4b46931
Merge branch 'main' into main
tnemoz File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
# This code is part of a Qiskit project. | ||
# | ||
# (C) Copyright IBM 2024. | ||
# | ||
# This code is licensed under the Apache License, Version 2.0. You may | ||
# obtain a copy of this license in the LICENSE.txt file in the root directory | ||
# of this source tree or at http://www.apache.org/licenses/LICENSE-2.0. | ||
# | ||
# Any modifications or derivative works of this code must retain this | ||
# copyright notice, and modified files need to carry a notice indicating | ||
# that they have been altered from the originals. | ||
|
||
"""Types used by the qiskit-algorithms package.""" | ||
|
||
from typing import Any, List, Protocol, Union | ||
|
||
from qiskit import QuantumCircuit | ||
|
||
_Circuits = Union[List[QuantumCircuit], QuantumCircuit] | ||
|
||
|
||
class Transpiler(Protocol): | ||
"""A Generic type to represent a transpiler.""" | ||
|
||
def run(self, circuits: _Circuits, **options: Any) -> _Circuits: | ||
"""Transpile a circuit or a list of quantum circuits.""" | ||
pass |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
You can use
list
in 3.8 - which arguably is better and what we have tried to do elsewhere. You do need to add a future import to have that workfrom __future__ import annotations
. You can see examples in this repo e.g. in this file which does that and uses it https://github.com/qiskit-community/qiskit-algorithms/blob/main/qiskit_algorithms/time_evolvers/pvqd/pvqd_result.py Though I am not sure here with custom types defs though as I see Union and elsewhere we try and use the|
nowadays instead.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.
I initially went for
but then
make lint
complained with:which is why I went for
Union
instead of|
. Or is there a workaround I don't know of?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.
That future import is needed for
|
as well - but if you had that import then as I mentioned I was not not sure here as I recall having issues when defining types before using these newer aspects where we had to have them defined with the former Typing constructs. If you search in this repo you will find just a few Union hits where they are pretty much limited to being used in type defines like this - I also see List being used in the ones here so it may well be the same issue with that. I commented mostly as a saw the commit that changed things to fix it for lint in 3.8 from list to List that was all the change I did not see a from future import being removed which is needed to type things this newer way in 3.8.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.
One further comment around 3.8. Qiskit has already deprecated support for Python 3.8 and support will be removed in 1.3.0 which is due in Nov. Python 3.8 EOL is Oct this year so among the changes here it could be a choice to drop 3.8 support too along with these changes. To drop this would evidently also include dropping it from CI tests etc and bumping things to run at 3.9 min.
In talking about versions I will note 3.13 is planned to be available beginning of Oct. There is an issue on Qiskit Qiskit/qiskit#12903 to support this so again depending on timeline.... but hopefully that's just adding it as supported and to CI to test when dependencies are there and it just works!