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

[WIP] replace bit_tools.iter_bits with classical_sim.ints_to_bits #1032

Closed
wants to merge 3 commits into from

Conversation

charlesyuan314
Copy link
Contributor

This is just a first attempt at #811 to gather feedback and to see what tests fail.

Known issues:

  • iter_bits still exists even though it is entirely redundant.
  • iter_bits no longer supports sign.
  • iter_bits no longer raises ValueError for too-wide inputs.

This is merely a first attempt for feedback gathering purposes and to see what tests fail.
@charlesyuan314 charlesyuan314 marked this pull request as draft May 31, 2024 00:13
@charlesyuan314
Copy link
Contributor Author

@mpharrigan @tanujkhattar @fdmalone Do you think this is a sane approach?

P.S. Looks like I can't directly request a PR review without being a member of the quantumlib org.

@fdmalone
Copy link
Collaborator

I think this makes sense. The bigger migration might be to just replace iter_bits with the corresponding to_bits methods from the register data types(e.g. https://github.com/quantumlib/Qualtran/blob/main/qualtran/_infra/data_types.py#L215) which will handle the signed-ness etc. But maybe out of scope for this PR.

@mpharrigan
Copy link
Collaborator

Yeah, since this one of the issues with this consolidation (as currently presented) is that it drops the data-type-specific configuration options (signedness, input validation...) that are now encoded in the QDType implementations (which didn't exist when all these iter_bits functions were first introduced).

Can you find where these methods are used within the library and see if it would be sensible to replace them with e.g. QUInt(...).to_bits(x)? And maybe port these free functions to use the qdtype methods under-the-hood but with a backwards-compatible interface

@charlesyuan314
Copy link
Contributor Author

Closing as I'm currently working on a PR to replace the bit_tools functions with QDType implementations as @mpharrigan suggested.

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