Add FillMaps #113
Add FillMaps #113
Conversation
Codecov Report
@@ Coverage Diff @@
## master #113 +/- ##
=======================================
Coverage 99.78% 99.79%
=======================================
Files 13 14 +1
Lines 950 988 +38
=======================================
+ Hits 948 986 +38
Misses 2 2
Continue to review full report at Codecov.
|
src/LinearMaps.jl
Outdated
| LinearMap(λ::Number, M::Int, N::Int) = FillMap(λ, (M, N)) | ||
| LinearMap(λ::Number, dims::Dims{2}) = FillMap(λ, dims) |
There was a problem hiding this comment.
I think the only somewhat controversial things are these constructors. They are formally unambiguous, but are they unambiguous enough for users (sufficiently distinct from UniformScalinMap constructor?)? Or should we export the FillMap construct and not include these?
There was a problem hiding this comment.
As this is one of few the LinearMap types that you want to construct explicitly, I guess it would make sense to export the FillMap constructor explicitly. It's somewhat asymmetric that FunctionMap is then still constructed via LinearMap. Just using LinearMap(scalar, ...) is also fine by me, no strong preference.
I do wonder what this is used for though :-) ?
There was a problem hiding this comment.
I don't know either what this is used for, but (a) there is an entire package (FillArrays.jl) devoted to such arrays, and (b) I do notice that our simple imlementation of matvecmul and matmatmul is even faster than theirs.
And yes, I agree that this is so specific that it is perhaps better to have an explicit constructor. It was more "sentimentality" that made me consider keeping the "one constructor for all types" status.
dkarrasch
force-pushed
the
dk/fillmap
branch
3 times, most recently
from
4d20666 to
12b7711
Compare
adjust the documentation & write a docstring
write a
showmethod specialization (after Introduce specific show methods, add Base.parent overload #87 is included)decide on constructor