-
-
Notifications
You must be signed in to change notification settings - Fork 327
Initial GPU support #1967
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
Merged
d-v-b
merged 51 commits into
zarr-developers:v3
from
akshaysubr:gpu-buffer-implementation
Aug 30, 2024
Merged
Initial GPU support #1967
Changes from 14 commits
Commits
Show all changes
51 commits
Select commit
Hold shift + click to select a range
22a5807
Initial implementation of a GPU version of Buffer and NDBuffer
akshaysubr d8cc79f
Adding cupy as an optional dependency
akshaysubr 4d2b8c7
Adding GPU prototype test
akshaysubr 36b1cb2
Adding GPU memory store implementation
akshaysubr 04001b4
Addressing comments
akshaysubr 74a13c4
Making GpuMemoryStore tests conditional on cupy being available
akshaysubr bdc0a24
Adding test checking that existing host memory codecs use the gpu_buf…
akshaysubr d900aa3
Reducing code and docs duplication
akshaysubr 0eca795
Formatting
akshaysubr d9ed6c4
Fixing silent rebase conflicts
akshaysubr 5405e38
Reducing code duplication in GpuMemoryStore
akshaysubr 2858701
Refactoring to an abstract Buffer class and concrete CPU and GPU impl…
akshaysubr 4e18098
Templating store tests on Buffer type
akshaysubr 35948d4
Changing imports to prevent circular dependencies
akshaysubr bd2a20b
Fixing unsafe calls to Buffer abstract methods in metadata.py and gro…
akshaysubr 828401f
Preventing calls to abstract classmethods of Buffer and NDBuffer
akshaysubr 02a6e9d
Fixing some more unsafe usage of Buffer abstract class
akshaysubr ff40d3c
Initial testing with cirun based GPU CI
akshaysubr e5cfd2f
Reverting to basic ubuntu machine image on GCP
akshaysubr d473a3d
Switching to cuda image from the docker registry
akshaysubr 2a2e399
Revert "Switching to cuda image from the docker registry"
akshaysubr b89ab9a
Revert "Reverting to basic ubuntu machine image on GCP"
akshaysubr c5a387d
Revert "Initial testing with cirun based GPU CI"
akshaysubr 72d172d
Adding pytest mark for GPU tests
akshaysubr 3db61bd
Updating GPU memory store test with gpu mark
akshaysubr 425c3f8
Adding GPU workflow that only runs GPU tests
akshaysubr 75b0ad7
First pass at fixing merge conflicts, still many changes needed
akshaysubr c8c7e6d
Formatting
akshaysubr 25a67ca
Fixing mypy errors in buffer code
akshaysubr ce7f5e2
Merging again with v3
akshaysubr ac061d9
Fixing errors in test_buffer.py
akshaysubr 523d8d5
Fixing errors in test_buffer.py
akshaysubr b559ee4
Fixing store test errors
akshaysubr 26a74f4
Fixing stateful store test
akshaysubr 7307833
Fixing config test
akshaysubr f6fddd9
Fixing group tests
akshaysubr 2b1fe14
Fixing indexing tests
akshaysubr abd135f
Manually installing cupy in the GPU workflow
akshaysubr 1db58e7
Ablating GPU test matrix and adding gpu optional dependencies to the …
akshaysubr 296bd02
Adding some more logging to debug GPU test failures
akshaysubr b33c887
Adding GA step to install the CUDA toolkit
akshaysubr c894f60
Merging with v3
akshaysubr e0da0fb
Adding a separate gputest hatch environment to simplify GPU testing
akshaysubr 07277af
Fixing error in cuda-toolkit step
akshaysubr 6e49e85
Downgrading to CUDA 12.4.1 in cuda-toolkit GA
akshaysubr 02c319c
Trying manual install of the CUDA toolkit
akshaysubr e82ddc1
Updating environment variables with CUDA installation
akshaysubr 7854ce9
Removing PATH env and setting it only through GITHUB_PATH
akshaysubr 9688ad6
Merge branch 'v3' into gpu-buffer-implementation
akshaysubr 3852c9f
Fixing issue from merge conflict
akshaysubr 2e8069c
Merge branch 'v3' into gpu-buffer-implementation
d-v-b File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
from zarr.buffer.core import ( | ||
ArrayLike, | ||
Buffer, | ||
BufferPrototype, | ||
NDArrayLike, | ||
NDBuffer, | ||
) | ||
from zarr.buffer.cpu import default_buffer_prototype | ||
|
||
__all__ = [ | ||
"ArrayLike", | ||
"Buffer", | ||
"NDArrayLike", | ||
"NDBuffer", | ||
"BufferPrototype", | ||
"default_buffer_prototype", | ||
] |
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
Oops, something went wrong.
Oops, something went wrong.
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.
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.
test failures are due to this method becoming abstract, while it still gets called in
to_buffer_dict
inmetadata.py
. Two things should change to fix this: first, I think maybe the order of the decorators here should be flipped to ensure thatBuffer.from_bytes
can't be called without an exception, and secondmetadata.py
needs to not be calling anyBuffer
methods.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.
I'm not sure we can flip the order of decorators here based on this snippet from the
abstractmethod
docs:I agree though that
metadata.py
shouldn't be calling anyBuffer
methods and should instead be callingprototype.buffer
methods ordefault_buffer_prototype.buffer
methods. Is there a preference between propagating aprototype
argument up the call stack or just usingdefault_buffer_prototype
since theseto_bytes
calls are not in the critical performance path?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.
Resolved the CI issues by tracking all calls to abstract classmethods of
Buffer
andNDBuffer
. Here are the main ones and the current solution:metadata.py
: switch to usingdefault_buffer_prototype.buffer
group.py
: switch to usingdefault_buffer_prototype.buffer
sharding.py
: switch to usingdefault_buffer_prototype.buffer
codecs/_v2.py
: switch to usingcpu.Buffer
andcpu.NDBuffer
since these are all explicitly CPU only.