Skip to content

Conversation

mtreinish
Copy link
Member

Summary

In #10120 we moved to using the Python stable C API for the qiskit binaries we build. In that PR we encountered a limitation with PyO3 at the time when using abi3 it was unable to convert a BigUInt into a Python int directly. To workaround this we side stepped the issue by generating a string representation of the integer converting that to python and then having python go from a string to a int. This has some performance penalty and also prevented parallelism because a GIL handle was needed to do the conversion. In PyO3 0.19.1 this limitation was fixed and the library can handle the conversion directly now with abi3 and this commit restores the code that existed in the marginalization module prior to #10120.

Details and comments

In Qiskit#10120 we moved to using the Python stable C API for the qiskit
binaries we build. In that PR we encountered a limitation with PyO3 at
the time when using abi3 it was unable to convert a BigUInt into a
Python int directly. To workaround this we side stepped the issue by
generating a string representation of the integer converting that to
python and then having python go from a string to a int. This has some
performance penalty and also prevented parallelism because a GIL handle
was needed to do the conversion. In PyO3 0.19.1 this limitation was
fixed and the library can handle the conversion directly now with abi3
and this commit restores the code that existed in the marginalization
module prior to Qiskit#10120.
@mtreinish mtreinish added Changelog: None Do not include in changelog Rust This PR or issue is related to Rust code in the repository labels Aug 21, 2023
@mtreinish mtreinish requested a review from a team as a code owner August 21, 2023 22:22
@qiskit-bot
Copy link
Collaborator

One or more of the the following people are requested to review this:

@coveralls
Copy link

Pull Request Test Coverage Report for Build 5932064108

  • 6 of 11 (54.55%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.04%) to 87.241%

Changes Missing Coverage Covered Lines Changed/Added Lines %
crates/accelerate/src/results/marginalization.rs 6 11 54.55%
Totals Coverage Status
Change from base Build 5930965687: -0.04%
Covered Lines: 74346
Relevant Lines: 85219

💛 - Coveralls

@jakelishman jakelishman added this pull request to the merge queue Aug 22, 2023
Merged via the queue into Qiskit:main with commit 4c88f07 Aug 22, 2023
SameerD-phys pushed a commit to SameerD-phys/qiskit-terra that referenced this pull request Sep 7, 2023
)

In Qiskit#10120 we moved to using the Python stable C API for the qiskit
binaries we build. In that PR we encountered a limitation with PyO3 at
the time when using abi3 it was unable to convert a BigUInt into a
Python int directly. To workaround this we side stepped the issue by
generating a string representation of the integer converting that to
python and then having python go from a string to a int. This has some
performance penalty and also prevented parallelism because a GIL handle
was needed to do the conversion. In PyO3 0.19.1 this limitation was
fixed and the library can handle the conversion directly now with abi3
and this commit restores the code that existed in the marginalization
module prior to Qiskit#10120.
@mtreinish mtreinish deleted the fix-abi3-biguint-int-conversion branch August 17, 2024 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: None Do not include in changelog performance Rust This PR or issue is related to Rust code in the repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants