Support ComponentArrays

[gdalle]

• In trace_input, ensure that the array of indices is has the same type as the input array xs
• Add tests for ComponentArrays
• Bump version to 0.6.1

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.17%. Comparing base (fe43799) to head (f48d3ff).
Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #146      +/-   ##
==========================================
- Coverage   91.84%   89.17%   -2.68%     
==========================================
  Files          37       38       +1     
  Lines        1484     1469      -15     
==========================================
- Hits         1363     1310      -53     
- Misses        121      159      +38     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[adrhill]

Looks like all test failures are on trace_input with matrix types like Diagonal and Symmetric, e.g.:

ArgumentError: Cannot set a non-diagonal index in a symmetric matrix
  Stacktrace:
    [1] setindex!
      @ /opt/hostedtoolcache/julia/1.10.4/x64/share/julia/stdlib/v1.10/LinearAlgebra/src/symmetric.jl:258 [inlined]
    [2] _setindex!
      @ ./abstractarray.jl:1426 [inlined]
    [3] setindex!
      @ ./abstractarray.jl:1396 [inlined]
    [4] macro expansion
      @ ./broadcast.jl:1004 [inlined]
    [5] macro expansion
      @ ./simdloop.jl:77 [inlined]
    [6] copyto!
      @ ./broadcast.jl:1003 [inlined]
    [7] copyto!
      @ ./broadcast.jl:956 [inlined]
    [8] materialize!
      @ ./broadcast.jl:914 [inlined]
    [9] materialize!
      @ ./broadcast.jl:911 [inlined]
   [10] trace_input(::Type{GradientTracer{IndexSetGradientPattern{Int64, BitSet}}}, xs::Symmetric{Float64, Matrix{Float64}}, i::Int64)
      @ SparseConnectivityTracer ~/work/SparseConnectivityTracer.jl/SparseConnectivityTracer.jl/src/interface.jl:21
   [11] trace_input
      @ ~/work/SparseConnectivityTracer.jl/SparseConnectivityTracer.jl/src/interface.jl:17 [inlined]
   [12] trace_function(::Type{GradientTracer{IndexSetGradientPattern{Int64, BitSet}}}, f::var"#_f#253"{typeof(pinv)}, x::Symmetric{Float64, Matrix{Float64}})

[adrhill]

I've opened a related issue in #147 describing the problem at hand for Diagonal.

We are going to have to add more trace_input methods for common matrix types.

[gdalle]

In the meantime I would rather merge (some version of) this, which is more correct, and possibly remove tests on Symmetric and Diagonal, than keep the current broken behavior?

[adrhill]

Absolutely don't remove tests!

I could live with you copying the previous implementation of trace_input as an additional method on Diagonal, Symmetric and Co, making the new one the default for AbstractArray.

[adrhill]

And then we fix those "new" methods and close #147 in a follow-up PR.

[gdalle]

The tests we have are a bit inadequate to say the least, because they didn't catch this reshape bug. And this bug in turn underlines that we don't even know what the right semantics are for tracing from a structured array 🤷‍♂️

[adrhill]

We currently just are too conservative with the sparsity pattern on Diagonal.

The way I see it, support for ComponentArrays is orthogonal to making patterns on Diagonal less conservative.

If you remove the tests and support for Diagonal entirely, that would put this repo in a temporarily breaking state for no reason. Users would run into the same error the tests run into. Avoiding this just requires 5 additional LOC which already exist.

[gdalle]

Not sure what fix you have in mind, can I let you show me? You say we can keep the current behavior for diagonal and friends, but in my view that behavior is broken.

[gdalle]

Perhaps we should just call parent before we do anything