Reverts #1557 #1561
Reverts #1557 #1561
Conversation
|
bors r+ |
| @@ -40,17 +40,16 @@ import Flux: activations | |||
| @test Dense(rand(Float16, 100,10), true).bias isa Vector{Float16} # creates matching type | |||
| @test_skip Dense(rand(Float16, 100,10), rand(100)).bias isa Vector{Float16} # converts to match | |||
|
|
|||
| @test_skip Dense(3,4; init=Base.randn, bias=true).bias isa Vector{Float64} | |||
| @test Dense(3,4; init=Base.randn, bias=true).bias isa Vector{Float64} | |||
There was a problem hiding this comment.
This should be a @test_skip right? The correct eltype passing is an accident here (cause bias = true uses zeros which happens to be Float64).
There was a problem hiding this comment.
No. This was in #1558, and it uses Flux.zeros (not Base.zeros) which produces Float32 by default.
julia> Flux.zeros(4)
4-element Array{Float32,1}:
0.0
0.0
0.0
0.0I agree that the test isn't explicit about its intent. This should be rewritten.
There was a problem hiding this comment.
Ah I see, okay we'll address it separately. Thanks!
There was a problem hiding this comment.
Reverting here is good, we should produce float64 bias when the weights produced are float64
There was a problem hiding this comment.
we should produce float64 bias when the weights produced are float64
No
We should do as we are told by the user. Flux is meant to be hackable. While it is valid to have defaults, it is required to have overrides at different levels.
There was a problem hiding this comment.
I'm referring to the specific case tested by the line we are commenting here. When doing Dense(3,4; init=Base.randn, bias=true) the bias should be float64
|
bors r+ |
|
Okay so do I need to hit approve or is |
|
Build succeeded: |
Reverts #1557