Skip to content

Conversation

@haampie
Copy link
Contributor

@haampie haampie commented Mar 19, 2020

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 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
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

linear algebra Linear algebra

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Generic 5 arg mul! promotes Int32 to Int64

3 participants