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

Heirarchical domains #189

Open
peterbell10 opened this issue Jul 16, 2019 · 21 comments
Open

Heirarchical domains #189

peterbell10 opened this issue Jul 16, 2019 · 21 comments

Comments

@peterbell10
Copy link
Collaborator

@peterbell10 peterbell10 commented Jul 16, 2019

As discussed in scipy/scipy@7acf8ba#issuecomment-510888182

The domain a.b.c should be treated as a root domain a with sub-domains a.b and a.b.c. This means that calling a function in the leaf domain (a.b.c) should call backends registered to any of the super-domains, walking up the tree.

I think it would also be useful to be able to disable this on a per sub-domain basis so that the end-user can have more control over the backends being called and so they can fail-fast when not calling into the backend they expect.

@hameerabbasi

This comment has been minimized.

Copy link
Contributor

@hameerabbasi hameerabbasi commented Jul 16, 2019

By the user, do you mean the author of a given backend (the person who writes __ua_function__) or the API author (the person who writes the multimethods)? A simple solution would be to expose the domain on the multimethod or C-level _Function object. I can do this if needed.

@hameerabbasi

This comment has been minimized.

Copy link
Contributor

@hameerabbasi hameerabbasi commented Jul 16, 2019

Or do you mean immediately writing said code for hierarchical domains? I can do that too.

@peterbell10

This comment has been minimized.

Copy link
Collaborator Author

@peterbell10 peterbell10 commented Jul 16, 2019

By user, I mean the person installing backends and calling multimethods.

Edit: "End-user" in the uarray documentation.

@hameerabbasi

This comment has been minimized.

Copy link
Contributor

@hameerabbasi hameerabbasi commented Jul 16, 2019

How about a ua.skip_domain(domain: str, subdomains: bool=True) context manager?

@hameerabbasi

This comment has been minimized.

Copy link
Contributor

@hameerabbasi hameerabbasi commented Jul 16, 2019

I'd still prefer the correct method: Don't depend on the name of the multimethod, it's a flaky decision at best. Also check the __module__ within __ua_function__.

@peterbell10

This comment has been minimized.

Copy link
Collaborator Author

@peterbell10 peterbell10 commented Jul 16, 2019

I'd still prefer the correct method: Don't depend on the name of the multimethod

This has nothing to do with it, it's about giving the end-user more control over what backend gets called. e.g. if I install the pyfftw backend to numpy.scipy.fft, I might want to make absolutely sure that it's the one being called and not the scipy default.

@hameerabbasi

This comment has been minimized.

Copy link
Contributor

@hameerabbasi hameerabbasi commented Jul 16, 2019

This has nothing to do with it, it's about giving the end-user more control over what backend gets called. e.g. if I install the pyfftw backend to numpy.scipy.fft, I might want to make absolutely sure that it's the one being called and not the scipy default.

This is already possible with the set_global_backend(..., only=True) and set_backend(..., only=True), but as I understand currently you'd like to limit backends to those with domains of numpy.scipy.fft?

@peterbell10

This comment has been minimized.

Copy link
Collaborator Author

@peterbell10 peterbell10 commented Jul 16, 2019

This is already possible with the set_global_backend(..., only=True)

If you do that then registered backends aren't usable.

@hameerabbasi

This comment has been minimized.

Copy link
Contributor

@hameerabbasi hameerabbasi commented Jul 16, 2019

Currently subdomains have exact matching, so there is no point in such functionality... Unless you'd like a dummy function that'll gain this functionality once hierarchical domains are implemented?

@peterbell10

This comment has been minimized.

Copy link
Collaborator Author

@peterbell10 peterbell10 commented Jul 16, 2019

Currently subdomains have exact matching, so there is no point in such functionality...

That's why it's in the same issue :)

@hameerabbasi

This comment has been minimized.

Copy link
Contributor

@hameerabbasi hameerabbasi commented Jul 16, 2019

Okay. We'll add this once hierarchical domains are implemented.

@hameerabbasi

This comment has been minimized.

Copy link
Contributor

@hameerabbasi hameerabbasi commented Sep 5, 2019

cc @peterbell10 @rgommers

The reason I want numpy backends to be tried before numpy.fft for example:

  • dask.array ought to become a numpy backend. It should take precedence over MKL FFT or similar, otherwise we'd have GPU stuff happening on the CPU, non-sparse stuff happening or distributed stuff happening locally, etc.

The reason I'd like this over a list of domains:

  • There's no need to know the list of domains beforehand.
@peterbell10

This comment has been minimized.

Copy link
Collaborator Author

@peterbell10 peterbell10 commented Sep 5, 2019

dask.array ought to become a numpy backend. It should take precedence over MKL FFT

I don't think that they should be in competition with each other. If you pass in a dask array to the mkl_fft backend it should return NotImplemented and then the dask backend would be called anyway.

In your example, I think numpy.fft should be tried first because I am assuming that a more specialised backend would only be installed if it was in some way preferred over the less specialised version e.g. MKL FFT should be preferred over the default numpy FFTs because it's faster. If it's done as you suggest then installing mkl_fft and the default numpy backend wouldn't ever call into mkl_fft.

@hameerabbasi

This comment has been minimized.

Copy link
Contributor

@hameerabbasi hameerabbasi commented Sep 5, 2019

In your example, I think numpy.fft should be tried first because I am assuming that a more specialised backend would only be installed if it was in some way preferred over the less specialised version e.g. MKL FFT should be preferred over the default numpy FFTs because it's faster. If it's done as you suggest then installing mkl_fft and the default numpy backend wouldn't ever call into mkl_fft.

Fair enough... You've convinced me. 😄

@hameerabbasi

This comment has been minimized.

Copy link
Contributor

@hameerabbasi hameerabbasi commented Sep 7, 2019

@peterbell10 In light of the discussion we had yesterday, would it be better to just reverse the order of walking the tree? i.e. root to leaf, rather than leaf to root? It seems that's the cleanest solution here.

@peterbell10

This comment has been minimized.

Copy link
Collaborator Author

@peterbell10 peterbell10 commented Sep 7, 2019

To be clear, when you say "root to leaf" do you mean e.g. try numpy first then numpy.fft? I thought we had agreed on the other way round.

@hameerabbasi

This comment has been minimized.

Copy link
Contributor

@hameerabbasi hameerabbasi commented Sep 7, 2019

I racked my brain and I can't seem to find a situation where trying numpy.fft first would be useful.

@peterbell10

This comment has been minimized.

Copy link
Collaborator Author

@peterbell10 peterbell10 commented Sep 7, 2019

What about mkl_fft?

@hameerabbasi

This comment has been minimized.

Copy link
Contributor

@hameerabbasi hameerabbasi commented Sep 7, 2019

That should be tried ideally after CuPy, Dask, or PyData/Sparse.

@peterbell10

This comment has been minimized.

Copy link
Collaborator Author

@peterbell10 peterbell10 commented Sep 7, 2019

But it will never be tried because it would just call the numpy backend.

@rgommers

This comment has been minimized.

Copy link
Member

@rgommers rgommers commented Sep 7, 2019

If mkl_fft is tried first, it will anyway say NotImplemented for CuPy/Dask/Sparse arrays right? So it's fine to try mkl_fft first? More specialized to less specialized makes sense to me as the general principle here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.