-
Notifications
You must be signed in to change notification settings - Fork 912
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
[FEA] Improve support or failure modes for numpy and other libraries with C APIs in cudf.pandas #15397
Comments
This is directly related to #15910, so I should also try to catch cases in our proxying scheme where we fallback to Pandas due to NumPy (or other libraries with C APIs).
I think once we run the pandas test suite with cudf.pandas debugging modes turned on we should get a better of what kinds of failures we're running in to. |
Apart of #15397. Closes #14537. Creates `ProxyNDarray` which inherits from `np.ndarray`. Authors: - Matthew Murray (https://github.com/Matt711) - GALI PREM SAGAR (https://github.com/galipremsagar) Approvers: - GALI PREM SAGAR (https://github.com/galipremsagar) - Matthew Roeschke (https://github.com/mroeschke) URL: #16286
With the solution in #16601, we are actually returning a (subclass of a) numpy array. Our primary goal was to make instance checks |
Thanks for not losing track of this issue @vyasr. Your summary sounds correct to me. We do have a few tests that verify that
I think doing both of those things will go far, but it may not cover the case where a user writes a function in pure C, compiles it so a shared object, and uses it in python. It may be overkill, but maybe we should add a few tests where the custom function is in pure C? |
I don't think that we need to go that far. I think your points 1 and 2 are great and would be sufficient. I would love to see a catboost test if you could throw something together. As long as we cover enough third-party libraries I'll feel good about us covering a wide range of possible uses of the C API. The case you're describing is fundamentally no different than any of the others. |
Currently we proxy numpy.ndarray to ensure that cupy arrays are produced instead where possible. However, this behavior only partially addresses the possible ways to interact with numpy. Similarly to how we handle pandas in cudf.pandas, we may want to install a proxied library for numpy itself in such cases to ensure that we get the desired behavior.
Unfortunately, this is far more challenging with numpy than with pandas due to the fact that numpy exposes a C API. This issue is not unique to numpy, but is also present for other libraries (like torch) that rely on being able to translate Python objects from standard vocabulary types like numpy arrays down to C representations to leverage in their own C code. In general, our approach for proxying is incompatible with code that relies on converting Python objects into their C representations since we cannot mimic the latter, only the former.
We should collect cases where we observe failures like this and see if we can, at minimum, improve the ways in which the code fails. If possible, we can also try to come up with more robust strategies for consuming libraries to use such that they won't accidentally go down such bad code paths with cudf.pandas objects.
In an ideal world, we would come up with a solution that actually enables support for such use cases, but at present I don't see how we could manage doing so without doing something crazy like hooking every function call with
sys.setprofile
, and even if that worked (I'm not sure that it would) the cure might be worse than the disease because we'd most likely slow down every function call in Python enough to overcome any gains from using cudf.pandas.The text was updated successfully, but these errors were encountered: