Skip to content
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

Matrix * Vector uses convert with promote_op, causes issues on 1.1+ #632

Closed
ararslan opened this issue May 20, 2019 · 1 comment · Fixed by JuliaLang/julia#32097
Closed

Comments

@ararslan
Copy link
Member

I don't have a minimal reproducing example at the moment, since this arose in private code, but consider the following situation: Say you have a Matrix{Float64} called A and a Vector{K} called x, where K is an abstract type. Various operations such as + and * are defined for K to return specific subtypes. The subtypes do not convert between each other. A * x should work; all of the necessary operations are defined.

With Julia 1.0, we get a Vector{Any} from A * x. Not ideal but fine. On 1.1, we get an error, saying it can't convert x's elements to a certain type. The issue seems to boil down to this difference:

On 1.0:

julia> Base.promote_op(LinearAlgebra.matprod, Float64, K)
Any

1.1:

julia> map(eltype, x)
3-element Array{DataType,1}:
 K0          
 K4
 K0   

julia> Base.promote_op(LinearAlgebra.matprod, Float64, K)
Union{K1, K2, K3}

LinearAlgebra then tries to do convert(AbstractArray{Union{K1, K2, K3}, x), which fails, since it can't convert x's elements to that type.

As of JuliaLang/julia#29739, promote_op is just a wrapper atop _return_type, which is returning the correct answer, but is not appropriate for this use.

@JeffBezanson
Copy link
Member

Great example --- inferring a more specific type can actually break things when return_type is used, in addition to the more obvious examples of inferring a less specific type breaking things.

ararslan referenced this issue in JuliaLang/julia May 20, 2019
This method entirely duplicates the one below it with the exception that
it also does a `convert` on one of the inputs with the result of
`promote_op`, which doesn't make a whole lot of sense in some cases. By
virtue of calling `mul!`, strided arrays that go through the abstract
array method will still hit the same optimized methods that use BLAS.

Fixes #32092.
ararslan referenced this issue in JuliaLang/julia May 22, 2019
This restricts the element type of the input vector to be `<:Real` and
only converts in case the promoted type is concrete.

Fixes #32092.
JeffBezanson referenced this issue in JuliaLang/julia May 23, 2019
This restricts the element type of the input vector to be `<:Real` and
only converts in case the promoted type is concrete.

Fixes #32092.
KristofferC referenced this issue in JuliaLang/julia May 23, 2019
This restricts the element type of the input vector to be `<:Real` and
only converts in case the promoted type is concrete.

Fixes #32092.

(cherry picked from commit 587cb82)
KristofferC referenced this issue in JuliaLang/julia Aug 26, 2019
This restricts the element type of the input vector to be `<:Real` and
only converts in case the promoted type is concrete.

Fixes #32092.

(cherry picked from commit 587cb82)
KristofferC referenced this issue in JuliaLang/julia Aug 26, 2019
This restricts the element type of the input vector to be `<:Real` and
only converts in case the promoted type is concrete.

Fixes #32092.

(cherry picked from commit 587cb82)
KristofferC referenced this issue in JuliaLang/julia Feb 20, 2020
This restricts the element type of the input vector to be `<:Real` and
only converts in case the promoted type is concrete.

Fixes #32092.

(cherry picked from commit 587cb82)
@KristofferC KristofferC transferred this issue from JuliaLang/julia Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants