Implementation of std and var.
#327
Conversation
|
Thank you for submitting a PR for this @alxmrs!
That's how I envisaged doing this (similar to Dask). The way you have it at the moment has two separate reductions which will be less efficient than one (and reductions in Cubed are quite inefficient at the moment, something we want to remedy in e.g. #284). Also I'm not sure if there are any differences in numerical stability between the two approaches. |
|
It would be good to get std/var into Cubed. I found an implementation that we could try here (see #29 (comment)) - would you be interested in having a go at that @alxmrs? I wonder whether we should merge this in the meantime though? |
| @@ -152,3 +153,12 @@ def sum(x, /, *, axis=None, dtype=None, keepdims=False): | |||
| keepdims=keepdims, | |||
| extra_func_kwargs=extra_func_kwargs, | |||
| ) | |||
|
|
|||
|
|
|||
| def var(x, /, *, axis=None, keepdims=False): | |||
There was a problem hiding this comment.
This doesn't match the signature in the specification since it's missing a correction parameter. Similarly for std.
| @@ -563,6 +563,74 @@ def test_sum_axis_0(spec, executor): | |||
| assert_array_equal(b.compute(executor=executor), np.array([12, 15, 18])) | |||
|
|
|||
|
|
|||
| def test_var(spec, executor): | |||
There was a problem hiding this comment.
Can you parameterize all these tests for var for different axes, and for keepdims? There are other tests in the test suite that do this.
|
Hey Tom. I don’t want to block you in the implementation of fundamental
routines. How about I try to get this merged in the next three days/EOW,
and if I can’t find the time, we close the PR? I appreciate how you’re
trying to incorporate my patches to cubed; thanks, I feel included.
…On Mon, Sep 23, 2024 at 9:49 PM Tom White ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In cubed/array_api/statistical_functions.py
<#327 (comment)>:
> @@ -152,3 +153,12 @@ def sum(x, /, *, axis=None, dtype=None, keepdims=False):
keepdims=keepdims,
extra_func_kwargs=extra_func_kwargs,
)
+
+
+def var(x, /, *, axis=None, keepdims=False):
This doesn't match the signature in the specification
<https://data-apis.org/array-api/latest/API_specification/statistical_functions.html>
since it's missing a correction parameter. Similarly for std.
------------------------------
In cubed/tests/test_array_api.py
<#327 (comment)>:
> @@ -563,6 +563,74 @@ def test_sum_axis_0(spec, executor):
assert_array_equal(b.compute(executor=executor), np.array([12, 15, 18]))
+def test_var(spec, executor):
Can you parameterize all these tests for var for different axes, and for
keepdims? There are other tests in the test suite that do this.
—
Reply to this email directly, view it on GitHub
<#327 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AARXAB3V2ICY2Q2ROCXYXATZYAE5ZAVCNFSM6AAAAABOV77TMWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDGMRSGA4DMMZRGY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Thanks Alex - that sounds like a good plan! (Sorry I didn't suggest how to move forward on this before - you weren't blocking me!) |
Fixes cubed-dev#29. Here, I use existing cubed operations to implement `var` and `std`. Please let me know if I should reimplement the primitives as pure reductions.
for more information, see https://pre-commit.ci
Fixes #29. Here, I use existing cubed operations to implement
varandstd. Please let me know if I should reimplement the primitives as pure reductions.