array-api-compat icon indicating copy to clipboard operation
array-api-compat copied to clipboard

Wrap fft for dask

Open lithomas1 opened this issue 1 year ago • 7 comments

There's some failures relating to the expected dtype on older numpy versions.

lithomas1 avatar May 19 '24 04:05 lithomas1

The wrappers in common should already be handling the dtype difference. It looks like the norm keyword isn't working, though.

asmeurer avatar May 20 '24 18:05 asmeurer

Going to pick this up again this week.

Thanks for not closing.

lithomas1 avatar Aug 24 '24 14:08 lithomas1

This should be ready for a look now.

Tests are passing with dask main. (this should be mergeable whenever dask cuts its next release).

lithomas1 avatar Aug 24 '24 18:08 lithomas1

Does dask still require the wrappers here? The only thing they do for NumPy is fix upcasting for 32-bit types, which has been fixed in NumPy 2.0. If the next version of dask has the NumPy 2.0 behavior then no wrapping is necessary at all.

asmeurer avatar Sep 03 '24 21:09 asmeurer

Ah thanks, I'll go and wrap that.

Curious why it's only on fftfreq and not on fft

lithomas1 avatar Sep 25 '24 13:09 lithomas1

fftfreq creates an array from scalar inputs, so there is no device to infer.

fft takes arrays as input so can infer the device.

lucascolley avatar Sep 25 '24 13:09 lucascolley

I think I was actually wrapping stuff incorrectly all this time (I forgot to import the wrappers in init)

I'm going to give only wrapping fftfreq/rfftfreq a shot.

lithomas1 avatar Sep 28 '24 18:09 lithomas1

@asmeurer This should be ready for a look now.

Sorry for the long delay, had to wrangle with Github Actions/the __all__ tests for a while.

lithomas1 avatar Oct 14 '24 21:10 lithomas1

This looks good, and I think the existing tests should be checking all the code that's been added here, so if tests are passing I'm reasonably confident this is all correct.

asmeurer avatar Oct 15 '24 20:10 asmeurer