-
Notifications
You must be signed in to change notification settings - Fork 108
Python array API support #1683
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
Draft
Emvlt
wants to merge
116
commits into
odlgroup:master
Choose a base branch
from
Emvlt:python_array_api_support
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Python array API support #1683
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ab70b6f
to
c698bfa
Compare
…m the util module
…s into methods for clarity
lable_dtypes: static-method -> attribute and addition of the NUMPY_DTYPES module dict(key, np.dtype). Removal of the dtype parsing from the backend npy_ensor
Space class (arguments and return of parse_weighting)
…the two cn and rn helper functions
…hanged the astype method necessary to get the element method to work
…tion to the Tensor class
…Weighting class, five subclasses inheriting from it and 5 classes implementing the latter per backend, we are down to one class with one entry point
… array_API_support. The idea is to have the backend checks all defined in a single place rather than duplicating them
… precision of a space
- utility module: fixed a typo (flot64 instead of float64) - npy_tensors.py: changes to the get_dtype_identifier method to recover the string identifier from a numpy dtype. changes to the way the NUMPY_DTYPES dict is defined. We now use only the np.dtype() call rather than np.float32, np.float64... - base_tensors.py: changes to the astype method to accomodate for non-string input dtypes.
…p.vecdot and np.vdot
…. It is a list of tuples such as [('impl', 'cpu'), ('pytorch','cuda:0')]
…ed for inner products.
odl/space/weighting.py
Outdated
@@ -498,7 +498,7 @@ def _inner_default(x1, x2): | |||
# This could also be done with `np.vdot`, which has complex conjugation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment is now out of date. Fixed in d9575e8
…backends. Most of the functionality is handled by the Python Array API, which this class defers to. Additionally there is some meta-information, and the backends are registered so they can always be looked up by a simple `impl` string as customary. I found it easiest to formulate this as a dataclass, which is concise and clear yet allows for the central registry. Concrete backends are simply instances of the `ArrayBackend` class. An alternative would be to let them be subclasses. This is not really necessary, but would arguably make the syntax less crammed and allows for better documentation. It would be less appropriate in the sense that those subclasses would be _singleton classes_, and would need to be registered in a different way. Python does allow this (via decorators or metaclasses), but that would be much more arcane.
This will allow delegating much of the lookups to that class, meaning they do not need to be defined in implementation-specific tensor space classes anymore.
These are properties that do not pertain to a specific space with its mathematical meaning, but only to the computational backend that it happens to use for storing its internal data. This makes some use cases slightly more verbose, but I would say it is justified by the simplification of the interface to the anyways rather bloated `TensorSpace` class. In principle, the `array_namespace` method could also be removed from `TensorSpace`, but this is invoked so often that the cost/benefit ratio is less favourable.
…f `TensorSpace`. These methods can/should _only_ be used from the `__init__` methods, and never by users of a complete `TensorSpace` object. Also, for the most part they are not concerned with parsing anything.
This function does not do anything specific to a particular space, but is rather a property of the array backend. It is also useful without the context of an already defined space, indeed it can be particularly useful to find out how to initialize such a space in the first place.
This is another method that was in `TensorSpace` out of necessity, but really does not have anything to do with ODL / mathematics but only with backend-specific ways of handling dtypes.
It should not be possible to directly instantiate this class (only backend-specific or otherwise subclasses). Not all subclasses directly store any `data`, they may instead delegate this to another object that they store (like, in case of `DiscretizedSpaceElement`, another instance of a `Tensor` subclass).
…n the base class. `__real_dtype` may only be defined in a subclass, and should be accessed via a virtual method instead to avoid slicing problems.
…her than hard-coding one itself.
This is how it was in old ODL. There is a discussion to be had about this, since single precision does give much better performance on GPUs. The lower precision is unlikely to matter in the typical applications, though it could still trip up users particularly in purer maths contexts.
Idea/array backend class
…ighting for impl and device
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The aim of this pull request is to make ODL multi backend through the Python Array API.