search

willow-ahrens / Finch.jl

Sparse tensors in Julia and more! Datastructure-driven array programing language.
http://willowahrens.io/Finch.jl/
MIT License
157 stars 15 forks source link

Fix `SwizzleArray` broadcasting #473

Closed mtsokol closed 5 months ago

mtsokol commented 5 months ago

Hi @willow-ahrens,

In the eager mode, I think there's an issue with broadcast(fn, ::SwizzleArray, ::SwizzleArray). It complained about missing functions, but with them in place it fails when performing copyto!.

The test/test_issues.jl change reproduces the issue.

willow-ahrens commented 5 months ago

The issue here is more that the swizzle wasn't declaring a FinchStyle broadcast style. I added a definition for that.

In general, most of the "interface" overloads for Tensor (i.e. getindex, setindex, map, broadcast, reduce, copy), should also be defined for SwizzleArray, PermissiveArray, OffsetArray, etc. I think the right way forward on that is to add an abstract class like CompiledArray that everything can inherit from for the purposes of defining interfaces in terms of the Finch compiler.

The fixes I've added so far should be good for now though!

willow-ahrens commented 5 months ago

Also, I've chosen not to define length on Finch tensors because most of the tensors require cartesian indexing to work properly, and length tends to be used for linear indexing. If at some point we want to do a linear indexing + length PR we could add them though!

mtsokol commented 5 months ago

Thank you! In this version broadcast with SwizzleArrays works for me!

I think the error comes from copyto!(A, A_sw) line.

willow-ahrens commented 5 months ago

Oops! Yeah I forgot how julia Array cannot be resized

codecov[bot] commented 5 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 76.23%. Comparing base (8078944) to head (a563ead).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #473 +/- ## ========================================== + Coverage 76.21% 76.23% +0.01% ========================================== Files 92 92 Lines 8834 8839 +5 ========================================== + Hits 6733 6738 +5 Misses 2101 2101 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.