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

Consider using pybind for seamless operability between C++11 and python #1

Closed
paniash opened this issue Sep 2, 2021 · 7 comments
Closed

Comments

@paniash
Copy link
Contributor

paniash commented Sep 2, 2021

Hi Dan!

Now that pyqrack has been made with the intention of providing python bindings for qrack, might I suggest using the pybind library made specifically for this purpose. This will reduce a lot of work writing these bindings from scratch. :-)

@WrathfulSpatula
Copy link
Contributor

@paniash Thank you for the suggestion, and I'll consider it. I'm not really a Python programmer, first, and I wasn't aware that pybind might do the job more easily.

However, notice that, beyond Qrack itself, we have 0 dependencies; this is literally pure Python, at least. Qrack itself, when compiled without OpenCL, is pure C++11, also no linker or package dependencies, beyond std namespace. This is part of the core philosophy of Qrack, to absolutely minimize attack surface and any potential points of failure or deprecation in dependencies.

If this project grows, I'll definitely keep pybind in mind, (because this was honestly mind-numbing to code, last night). However, I'm sure you see the reasoning behind dependency-free pure Python, if the Qrack shared library interface doesn't change rapidly.

@paniash
Copy link
Contributor Author

paniash commented Sep 2, 2021

@WrathfulSpatula Sure! I'm always in favour of lesser dependencies! Just thought I'd let you know.

Thanks!

@paniash paniash closed this as completed Sep 2, 2021
@paniash
Copy link
Contributor Author

paniash commented Sep 2, 2021

Found another one: https://cppyy.readthedocs.io/en/latest/

@WrathfulSpatula
Copy link
Contributor

I'd certainly be open to alternative implementations, perhaps particularly less verbose ones, but I think, in basically one night's work, at least we also have a dependency-free shared library wrapper, now, alongside any others, whether it was slightly short-sighted in the "design phase." 😃

@WrathfulSpatula
Copy link
Contributor

@paniash Thank you for the suggestion. My first impulse, with Qrack, is always to first resort to pure language standard. However, after a little time testing the pure ctypes implementation, we lack exception handling that PyQrack needs. Specifically, when the OpenCL API returns errors, (hopefully intermittently,) our best recourse in C++ is to throw exception, but Python user code can't easily handle these low-level exceptions without a wrapper like via pybind11 to convert these C++ exceptions into ones that Python can handle.

I'm already looking into this, and it's going in for the next release. However, it'd be great if you had input or wanted to contribute code. Perhaps you could review the PR. (Thanks in advance!)

WrathfulSpatula added a commit to unitaryfund/qrack that referenced this issue Oct 9, 2021
@paniash
Copy link
Contributor Author

paniash commented Oct 9, 2021

@WrathfulSpatula Hi! I'll be happy to contribute when I find some time from my work. Thanks!

@WrathfulSpatula
Copy link
Contributor

I went ahead and actually wrote manual exception handling in the shared library C interface itself, a while ago, so error handling is no longer something we lack. I realized, while Python can handle exceptions in a shared library, this was the right choice for the shared library C interface as a self-standing piece of software in itself, as C++ exceptions don't exist in C, and so they should be handled before the layer at which we expose a C interface. Hence, if a shared library C interface will be reused, exception handling with a Python dependency is kind of moot, though it's a great feature to offer if a *.dll/*.so only exists to serve a single Python package!

So, this was a great suggestion that's still on my mind, but we already did the hard work, to avoid the dependency. I'm open to further discussion, but I think we can consider the issue ticket closed, for now.

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

No branches or pull requests

2 participants