Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Warn on use of property_set kwarg to BasePassManger.run #13821

Merged

Conversation

jakelishman
Copy link
Member

The transpiler's BasePass.__call__ implementation accepts a property_set kwarg that can be used to seed the property set for the run of that pass. Starting in Qiskit 2.0, PassManager.run will also have that behaviour, so this commit begins emitting a warning on the potential change.

The impact of this is expected to be minimal, as we are not aware of much use of the subclassing of the generic BasePassManager, and even less of passing arbitrary keyword arguments to the conversion functions.

Summary

Details and comments

This is the warning to enable to API change of #13820.

The transpiler's `BasePass.__call__` implementation accepts a
`property_set` kwarg that can be used to seed the property set for the
run of that pass.  Starting in Qiskit 2.0, `PassManager.run` will also
have that behaviour, so this commit begins emitting a warning on the
potential change.

The impact of this is expected to be minimal, as we are not aware of
much use of the subclassing of the generic `BasePassManager`, and even
less of passing arbitrary keyword arguments to the conversion functions.
@jakelishman jakelishman added Changelog: API Change Include in the "Changed" section of the changelog mod: transpiler Issues and PRs related to Transpiler labels Feb 10, 2025
@jakelishman jakelishman added this to the 1.4.0 milestone Feb 10, 2025
@jakelishman jakelishman requested a review from a team as a code owner February 10, 2025 18:25
@qiskit-bot
Copy link
Collaborator

One or more of the following people are relevant to this code:

  • @Qiskit/terra-core

Copy link
Contributor

@ElePT ElePT left a comment

Choose a reason for hiding this comment

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

LGTM, I just had a small comment to make sure I am understanding the changes.

qiskit/passmanager/passmanager.py Show resolved Hide resolved
Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
ElePT
ElePT previously approved these changes Feb 14, 2025
Copy link
Contributor

@ElePT ElePT left a comment

Choose a reason for hiding this comment

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

Alright, I think that the PR looks good.

Copy link
Contributor

@ElePT ElePT left a comment

Choose a reason for hiding this comment

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

That is a good idea, I prefer "_MISSING" to "_NOT_PRESENT". I wouldn't say that the issue is the double negation as much as having two "nots" one directly after the other and accidentally skipping one.

qiskit/passmanager/passmanager.py Outdated Show resolved Hide resolved
qiskit/passmanager/passmanager.py Outdated Show resolved Hide resolved
qiskit/passmanager/passmanager.py Outdated Show resolved Hide resolved
Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
Copy link
Contributor

@ElePT ElePT left a comment

Choose a reason for hiding this comment

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

Nice!

@ElePT ElePT added this pull request to the merge queue Feb 14, 2025
Merged via the queue into Qiskit:stable/1.4 with commit ea8dab9 Feb 14, 2025
16 checks passed
@jakelishman jakelishman deleted the passmanager-run-propertyset-kwarg branch February 14, 2025 15:32
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Changelog: API Change Include in the "Changed" section of the changelog mod: transpiler Issues and PRs related to Transpiler
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants