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

1.7-beta2: type instability for matrix multiplication #41489

Closed
j-fu opened this issue Jul 6, 2021 · 3 comments · Fixed by #41491
Closed

1.7-beta2: type instability for matrix multiplication #41489

j-fu opened this issue Jul 6, 2021 · 3 comments · Fixed by #41491
Labels
regression Regression in behavior compared to a previous version
Milestone

Comments

@j-fu
Copy link

j-fu commented Jul 6, 2021

Hi, I encountered a type instability in matrix multiplication

julia> A=Rational[1 1 1; 2 2 2; 3 3 3]
3×3 Matrix{Rational}:
 1//1  1//1  1//1
 2//1  2//1  2//1
 3//1  3//1  3//1

On 1.7-beta2:

julia> A*A
3×3 Matrix{Any}:
  6//1   6//1   6//1
 12//1  12//1  12//1
 18//1  18//1  18//1

On 1.6.1:

julia> A*A
3×3 Matrix{Rational}:
  6//1   6//1   6//1
 12//1  12//1  12//1
 18//1  18//1  18//1
@KristofferC KristofferC added this to the 1.7 milestone Jul 6, 2021
@JeffBezanson JeffBezanson added the regression Regression in behavior compared to a previous version label Jul 6, 2021
@simeonschaub
Copy link
Member

Note that in general, return type calculations with non-concrete types are always very fragile, because intermediary calls often have quite a few matching methods, so those calls will infer to Any, which in many cases means the overall result will also infer to Any. The reason this used to work is probably more by accident due to how +(::Rational, Rational) was implemented. It is easy enough to fix in this case though, so I will make a PR.

@j-fu
Copy link
Author

j-fu commented Jul 6, 2021

Ah ok, so to be on the safe side one should start with a concrete type in the first place.

@simeonschaub
Copy link
Member

Yeah, that's always better when you have the choice.

aviatesk pushed a commit that referenced this issue Jul 7, 2021
* fix #41489: inference of `+(::Rational, Rational)`

* implement review comments
KristofferC pushed a commit that referenced this issue Jul 19, 2021
* fix #41489: inference of `+(::Rational, Rational)`

* implement review comments

(cherry picked from commit cf4e1c4)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression Regression in behavior compared to a previous version
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants