Skip to content
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

Suppress qiskit algorithm globals deprecation messages #110

Merged
merged 1 commit into from
Dec 5, 2023

Conversation

woodsp-ibm
Copy link
Member

@woodsp-ibm woodsp-ibm commented Dec 4, 2023

Summary

The algorithm_globals function here in Qiskit Algorithms delegates its behavior to the equivalent logic in Qiskit as part of the migration support from algorithms in Qiskit that were migrated here. Since that time the function in Qiskit has been deprecated and deprecation messages added to alert the user to the migration. When this logic here was initially done those deprecation messages were not present, but since that time the way the logic is here causes users to see these deprecation messages which they should not. These messages also flooded the combined deprecation output from CI here. This PR suppresses these around the points of usage here so the messages are no longer raised.

Details and comments

@coveralls
Copy link

Pull Request Test Coverage Report for Build 7088705026

  • 14 of 15 (93.33%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.008%) to 90.578%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit_algorithms/utils/algorithm_globals.py 14 15 93.33%
Totals Coverage Status
Change from base Build 7005928554: 0.008%
Covered Lines: 6470
Relevant Lines: 7143

💛 - Coveralls

ElePT
ElePT previously approved these changes Dec 4, 2023
Copy link
Collaborator

@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.

Would it make sense to directly remove the import from qiskit instead of suppressing the warning?

Copy link
Collaborator

@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.

(Sorry I meant the previous review as a comment, not an approval)

@woodsp-ibm woodsp-ibm dismissed ElePT’s stale review December 4, 2023 19:30

The approval was unintended as per above comment

@woodsp-ibm
Copy link
Member Author

I had delegated to the qiskit.utils algorithm_globals as a migration path if all that was done was to change the import from qiskit.algorithms to qiskit_algorithms, as was outlined earlier, that users code, tests etc., using algorithms that depended on random function would still work as before. The logic here falls back to equivalent local code if its not able to import the qiskit.utils function and I had imagined that the local code would be the only code existing going forwards. The the fallback is seamless, for when the qiskit,utils function does not exist. Now the latest qiskit the qiskit.utils algorithm globals issues deprecation messages to alert users they need to change their usage of that if that was not clear initially. In Qiskit 1.0.0 users will be obligated to alter the code if they had not before - I had imagined that would be the time to remove the use/delegation of the qiskit.utils code. It could be done earlier if desired but if users have not yet migrated the results would change for any algorithms having random function use here. I figured maybe it should stay for a while longer hence the suppression of the deprecation messages that occur when it is delegating to the qiskit.utils function since they should not be visible if the users are using the algorithm_globals from here.

Copy link
Collaborator

@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.

Sounds good, I guess that I am getting a bit impatient with deprecations and removals hahaha

@ElePT ElePT merged commit fa022c3 into qiskit-community:main Dec 5, 2023
15 checks passed
@woodsp-ibm woodsp-ibm deleted the alg_global_dep branch December 5, 2023 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants