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

Fix generic mul accidental type promotion Int32 -> Int64 #35164

Merged

Conversation

haampie
Copy link
Contributor

@haampie haampie commented Mar 19, 2020

Fixes #35163.

Copy link
Member

@dkarrasch dkarrasch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Did you check whether there are more unintended 0s (instead of zeros) in similar contexts?

@dkarrasch dkarrasch added the domain:linear algebra Linear algebra label Mar 19, 2020
@haampie
Copy link
Contributor Author

haampie commented Mar 19, 2020

matmul.jl is clear of 0 and 1 literals in computations as far as I can see.

@haampie
Copy link
Contributor Author

haampie commented Mar 19, 2020

😕 the current generic_mul! is really broken it seems :(

Type deduction for the output vector is way too generic:

julia> Real[1 2] * Real[1.5; 2.0]
1-element Array{Any,1}:
 5.5

vs

julia> Real[1 2] .* Real[1.5; 2.0]
2×2 Array{Float64,2}:
 1.5  3.0
 2.0  4.0

The test failure is because I'm using zero(R) where R === Any. But there's 3 other places where this is used at the moment that apparently aren't covered by tests.

Not sure what to replace zero(R) with at the moment.

@KristofferC
Copy link
Sponsor Member

Not sure what to replace zero(R) with at the moment.

Does false work?

@dkarrasch
Copy link
Member

I'll rerun CI (the only failure seems unrelated) and merge if everything runs smoothly.

@dkarrasch dkarrasch closed this Mar 24, 2020
@dkarrasch dkarrasch reopened this Mar 24, 2020
@dkarrasch dkarrasch merged commit 3df8899 into JuliaLang:master Mar 24, 2020
@haampie haampie deleted the fix-generic-mul-accidental-type-promotion branch March 24, 2020 15:49
oxinabox pushed a commit to oxinabox/julia that referenced this pull request Apr 8, 2020
ravibitsgoa pushed a commit to ravibitsgoa/julia that referenced this pull request Apr 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Generic 5 arg mul! promotes Int32 to Int64
3 participants