search

unitaryfund / pyqrack

Pure Python bindings for the pure C++11/OpenCL Qrack quantum computer simulator library
MIT License
16 stars 8 forks source link

Minor bug fixes. #18

Closed Zshan0 closed 2 years ago

Zshan0 commented 2 years ago

There were a few minor bugs that went unnoticed, a few more of them persist that require discussion that can be continued on this thread before it is merged with the main branch.

WrathfulSpatula commented 2 years ago

@Zshan0 This is great! My only particular point to add is that dump_callback() and dump_ids_callback() should not have the self parameter, because they are specifically C callbacks, called from the Python code by the underlying C++ Qrack library itself, believe it or not. If you could revert just those two self parameters, we'll get this right in!

Zshan0 commented 2 years ago

I'm suggesting a few changes that could be discussed and bundled together in this PR itself.

  1. Wildcard imports are not recommended from libraries, [this]() is a minor fix that could be issued as a good first issue.
  2. I'm not fully sure, but I believe that permutation_expectation here must return the value result. The same applies for joint_ensemble_probability. If that is the correct behavior, I'll just add it to this PR.

Another suggestion that I would make is that the upcoming unitary hack is a good opportunity to add tests/CI for pyqrack :D

WrathfulSpatula commented 2 years ago

@Zshan0 Correct, permutation_expectation() and joint_ensemble_probability() should return their result, after the shared C library interface is polled for exception. As for the ctypes wildcard import, you can go ahead and import the explicit ctypes native type classes that we use, if you like.

Zshan0 commented 2 years ago

I can do import ctypes and make changes from c_ulonglong to ctypes.c_ulonlong rather than importing all the required classes, whatever you think would be better :)

WrathfulSpatula commented 2 years ago

Honestly, you're probably more aware of Python standards than I am, so I leave the choice for ctypes to your best judgement! (I'm basically a C++ developer providing an interface, in this case.)