-
-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
generic matmul depends on zero #194
Comments
Similar in spirit: JuliaLang/julia#8319 JuliaLang/julia#9276 |
See if you like a potential fix in JuliaLang/julia#10622. |
@andreasnoack did this ever get resolved? |
Still needed. Bit of a confusing gotcha in every demo that tries to show generic linear algebra capabilities. |
At least the OP example works now on master. |
This doesn't error anymore. |
It does with any user defined type though. Whatever workaround was added for Any is apparently not always applicable? |
And where is it tested? |
I added a test in JuliaLang/julia@351e8af but I guess that doesn't cover the demo example you tried? Please share the example if that is the case. |
julia> immutable DNum{T}
v::T
e::T
end
julia> Base.:+(x::DNum, y::DNum) = DNum(x.v + y.v, x.e + y.e)
julia> Base.:*(x::DNum, y::DNum) = DNum(x.v * y.v, x.v * y.e + y.v * x.e)
julia> DNum.(rand(1:10, 6, 4), rand(1:10, 6, 4)) * DNum.(rand(1:10, 4, 3), rand(1:10, 4, 3))
ERROR: MethodError: no method matching zero(::DNum{Int64})
Closest candidates are:
zero(::Type{Base.LibGit2.GitHash}) at libgit2\oid.jl:88
zero(::Type{Base.Pkg.Resolve.VersionWeights.VWPreBuildItem}) at pkg/resolve/versionweight.jl:80
zero(::Type{Base.Pkg.Resolve.VersionWeights.VWPreBuild}) at pkg/resolve/versionweight.jl:120
...
Stacktrace:
[1] _generic_matmatmul!(::Array{DNum{Int64},2}, ::Char, ::Char, ::Array{DNum{Int64},2}, ::Array{DNum{Int64},2}) at .\linalg\matmul.jl:506
[2] generic_matmatmul!(::Array{DNum{Int64},2}, ::Char, ::Char, ::Array{DNum{Int64},2}, ::Array{DNum{Int64},2}) at .\linalg\matmul.jl:478
[3] *(::Array{DNum{Int64},2}, ::Array{DNum{Int64},2}) at .\linalg\matmul.jl:141 |
Comment out the promote_rule and convert definitions in julia> A = map(ModInt{13}, rand(0:12, 5, 5))
5×5 Array{ModInts.ModInt{13},2}:
4 5 4 12 6
3 1 12 11 6
11 9 12 5 0
12 6 2 3 7
7 7 8 10 3
julia> A*A
ERROR: MethodError: Cannot `convert` an object of type Int64 to an object of type ModInts.ModInt{13}
This may have arisen from a call to the constructor ModInts.ModInt{13}(...),
since type constructors fall back to convert methods.
Stacktrace:
[1] _generic_matmatmul!(::Array{ModInts.ModInt{13},2}, ::Char, ::Char, ::Array{ModInts.ModInt{13},2}, ::Array{ModInts.ModInt{13},2}) at ./linalg/matmul.jl:506
[2] generic_matmatmul!(::Array{ModInts.ModInt{13},2}, ::Char, ::Char, ::Array{ModInts.ModInt{13},2}, ::Array{ModInts.ModInt{13},2}) at ./linalg/matmul.jl:478
[3] *(::Array{ModInts.ModInt{13},2}, ::Array{ModInts.ModInt{13},2}) at ./linalg/matmul.jl:141 |
I've looked a bit at the code and think that we should require that appropriate promotion and convert methods are defined for new arithmetic types if they are subtypes of something with some fallbacks defined like |
Right, fair enough. |
do you need to call zero if all dimensions are nonzero? |
Probably not, but it seems like a gotcha corner case – probably better to depend on zero up front so that people implement enough functionality that matmul works, full stop. |
You'd need to define conversion from |
Currently generic matmul uses
zero(eltype(a))
to fill the initial output array with zero values. When defining a new numeric type, this can be annoying (you may not have defined azero
method yet), but there are also real-world case where this causes matmul to fail, such as when you have anAny
array that just happens to contain numbers that you want to multiply:On the one hand, you really want this to be a
Float64
matrix for performance. But on the other hand, this really ought to work in any case.The text was updated successfully, but these errors were encountered: