search

JuliaLinearAlgebra / LinearMaps.jl

A Julia package for defining and working with linear maps, also known as linear transformations or linear operators acting on vectors. The only requirement for a LinearMap is that it can act on a vector (by multiplication) efficiently.
Other
303 stars 42 forks source link

Always use similar as fallback when resize throws in applying composite maps #221

Closed krcools closed 8 months ago

krcools commented 8 months ago

When using subtypes of AbstractVector that do not support resize! (such as BlockArrays.PseudoBlockVector), the memory optimisation in the _unsafe_mul! for composite maps result in errors.

I have changed _resize to allways fall back to memory allocation via similar, whereas previously this only happened when the argument was a SharedArrays.SharedVector.

codecov[bot] commented 8 months ago

Codecov Report

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

Comparison is base (4c95579) 99.68% compared to head (010ca9c) 99.68%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #221 +/- ## ========================================== - Coverage 99.68% 99.68% -0.01% ========================================== Files 22 22 Lines 1591 1589 -2 ========================================== - Hits 1586 1584 -2 Misses 5 5 ```

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

dkarrasch commented 8 months ago

LGTM. Do you have a quick test case at hand? I believe we have BlockArrays.jl in our test dependencies anyway.

krcools commented 8 months ago

I have added a test with a triple product of LinearMaps over BlockArrays in nontradaxes.jl

dkarrasch commented 8 months ago

It turns out that it requires four maps to hit that _resize path. 😄

dkarrasch commented 8 months ago

Thanks! That's a clear improvement, since otherwise there was no way to get this to work, right?

krcools commented 8 months ago

It turns out that it requires four maps to hit that _resize path. 😄

Thank you, I missed that the triple product would return before _resize got hit.

Thanks! That's a clear improvement, since otherwise there was no way to get this to work, right?

I think so, that was my motivation to submit the PR. A better solution is probably to implement _resize only for Vector, as I understand from the Julia coding recommendations that exception handling is rather expensive...