-
-
Notifications
You must be signed in to change notification settings - Fork 356
Add NDBuffer.empty #3191
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
Add NDBuffer.empty #3191
Conversation
7a9767b
to
9357820
Compare
9357820
to
e91b630
Compare
e91b630
to
3f6d18f
Compare
In zarr-developers#2904, I've found that `np.empty(...)` can be quite a bit faster than the `np.full` we use in places (https://github.com/zarr-developers/zarr-python/blob/baabf08d07e8518e3d37bd83c493a1d46ea7ac1d/src/zarr/core/buffer/cpu.py#L149. Though note that `np.empty` and `np.zeros` are about the same for the cases where we use that.) For the common case of chunk-aligned reads, the memset used by `np.full` or `np.zeros` should be unnecessary, because the codec pipeline will overwrite the memory anyway (or overwritten with `fill_value` if the store is missing the key). This adds a new method `NDBuffer.empty`, mirroring `np.empty`. To preserve backwards compatibility, it's *not* abstract. It delegates to the less efficient `NDBuffer.create`. But I've implemented it for our `gpu` and `cpu` buffers.
3f6d18f
to
0ed6bc1
Compare
dtype: npt.DTypeLike, | ||
order: Literal["C", "F"] = "C", | ||
) -> Self: | ||
return super(cpu.NDBuffer, cls).empty(shape=shape, dtype=dtype, order=order) |
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 is to test the base core.NDBuffer
implementation. This class inherits from cpu.NDBuffer
so we use super(cpu.NDBuffer, cls)
to get its parent's impelentation (the base class).
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 looks good to me! Thanks Tom.
Just a note: this might cause issues for people running the |
In #2904, I've found that
np.empty(...)
can be quite a bit faster than thenp.full
we use in places(
zarr-python/src/zarr/core/buffer/cpu.py
Line 149 in baabf08
np.empty
andnp.zeros
are about the same for the cases where we use that.)For the common case of chunk-aligned reads, the memset used by
np.full
ornp.zeros
should be unnecessary, because the codec pipeline will overwrite the memory anyway (or overwritten withfill_value
if the store is missing the key).This adds a new method
NDBuffer.empty
, mirroringnp.empty
. To preserve backwards compatibility, it's not abstract. It delegates to the less efficientNDBuffer.create
. But I've implemented it for ourgpu
andcpu
buffers.