Skip to content

Conversation

adriagarp
Copy link

Adds delegation for the cov function.

All backends (except PyData Sparse) have a cov implementation that accepts a single positional argument.

@lucascolley lucascolley added this to the 0.9.1 milestone Oct 2, 2025
@lucascolley lucascolley added enhancement New feature or request delegation labels Oct 2, 2025
@lucascolley lucascolley self-requested a review October 2, 2025 11:43
Copy link
Member

@lucascolley lucascolley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @adriagarp !

It looks like the tests are reporting some changes in the return dtypes. Could you write a summary about what changes here?

We may want to:

  • wrap the output from xp.cov to change dtypes
  • make a breaking change to match the behaviour of the underlying libraries

Either way, it would be nice to add return dtype information to the docstring, once we have decided what to do?

@adriagarp
Copy link
Author

adriagarp commented Oct 2, 2025

Sure! The test failures are as follows:

  • test_empty (jax and torch): for an empty tensor in jax, the actual result is array(-0.), while the expected value is array(nan). In torch, the test fails because of a UserWarning
  • test_combinationand test_xp (torch): torch defaults to float32, but the tests explicitly check for float64 results
  • test_device (torch): I'm quite lost with this error, since I've never worked with PyTorch. It's as follows:
    NotImplementedError: aten::equal: attempted to run this operator with Meta tensors, but there was no fake impl or Meta kernel registered. You may have run into this message while using an operator with PT2 compilation APIs (torch.compile/torch.export); in order to use this operator with those APIs you'll need to add a fake impl. Please see the following for next steps: https://pytorch.org/tutorials/advanced/custom_ops_landing_page.html
    Have you seen anything like this before? Any hints? I've tried reading the provided link but I couldn't understand why it's failing here and not in other similar tests

For the test_empty case, I would propose to check for empty arrays and return arran(nan) before calling cov, and document this in the docstring. What do you think?

For the float32 vs float64, what is the common approach here? Should we keep backend's behavior, or try to have an unified return dtype?

@lucascolley
Copy link
Member

lucascolley commented Oct 2, 2025

for an empty tensor in jax, the actual result is array(-0.), while the expected value is array(nan)

This sounds like we should open an upstream issue to ask whether they would consider changing the behaviour, and adding a pytest.mark.skip_xp_backend for now (see other uses in test_funcs).

In torch, the test fails because of a UserWarning

We can probably just add a line like

warnings.simplefilter("always", RuntimeWarning)
for UserWarning too

  • test_combinationand test_xp (torch): torch defaults to float32, but the tests explicitly check for float64 results

Could you adjust

x = xp.asarray([-2.1, -1, 4.3])
y = xp.asarray([3, 1.1, 0.12])
and
cov(xp.asarray([[0.0, 2.0], [1.0, 1.0], [2.0, 0.0]]).T, xp=xp),
to explicitly include dtype=xp.float64?

We could consider also adding a test_dtype that is like test_basic but parametrised by input dtype, but I won't require it here.

test_device (torch): I'm quite lost with this error,

Hmm, I am quite puzzled too. It seems like it isn't under our control, but rather torch.cov chokes whenever m has the meta device. Feel free to add a skip for now and we can open an issue to follow-up on that before we merge.

Adrián García Pitarch added 2 commits October 2, 2025 15:47
# Conflicts:
#	src/array_api_extra/__init__.py
#	src/array_api_extra/_delegation.py
@adriagarp
Copy link
Author

@lucascolley I've added explicit dtypes for all tests in TestCov, and created the two mentioned issues.

Let me know if anything's missing!

@lucascolley lucascolley self-requested a review October 2, 2025 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
delegation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants