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

Start of a prototype for string-based, very strict, type dispatching #2

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

seberg
Copy link

@seberg seberg commented Aug 30, 2024

This is an early (and very dirty) start, in case it is useful to get going. (Ping @eriknw)

The "example" is just the prototype_* modules, with the _example one being executable. I did not look into the entry-points, and of course this is all very raw:

  1. We can make the way we dispatch on types much smarter:
    • For speed
    • For backend order (potentially), this code doesn't even try to address that.
    • I had discussed with Eric, having types we expect/need to match, and types we understand can make sense. E.g. cucim may want to only kick in for cupy arrays, but understand numpy arrays. A better example maybe a dask.array backend which can understand cupy/numpy, but will never be picked for those.
  2. This is just string based, that can be relaxed of course.
  3. I added a WillNotHandle() return class to pass out information for rejection, since I liked that idea:
    • (We cannot "force" right now, in this way, I suspect that is OK for now.)
  4. I solved the parameter selection with strings + inspect, which I really like for simplicity.
    We need the signature fetching for it, but otherwise we can always optimize it.

Anyway, sharing in the hoping it might help. But please feel free to ignore if you have another start and are pushing forward there!

for t in types:
ident = get_identifier(t)

if ident in self.type_names:
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, this uses the string types, and @eriknw correctly pointed out (independent of this PR), that projects may sometimes be sloppy about which __module__ they define types.
I am not sure this is a problem, but we could work around it, with something like:

module, name = self.type_names[0]  # all of them of course
if module in sys.modules:
    t = getattr(module, name, None)  # ignore error?
    ...

I.e. I think rather than matching only by strings, we could check whether the type may be defined, by testing if the module exists.

@lucascolley
Copy link

For comparison, this is how we're currently doing basic type-delegation in scipy.ndimage: https://github.com/scipy/scipy/blob/main/scipy/ndimage/_support_alternative_backends.py, https://github.com/scipy/scipy/blob/main/scipy/ndimage/_dispatchers.py.

@seberg
Copy link
Author

seberg commented Sep 2, 2024

FWIW, this is a branch in skimage: scikit-image/scikit-image@main...seberg:backend-cucim-try that uses the state here. I added a backend for cucim there (for filters.median and filters.gaussian), that has to live in cucim and be auto-generated of course.
Difference in speed for a 1000x1000 NumPy vs. CuPy array is, 114us vs. 31ms on the machine I tried.

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.

2 participants