Flux.jl
Flux.jl copied to clipboard
Make `create_bias` a public API?
This was recently marked private in https://github.com/FluxML/Flux.jl/pull/1998, but looking at a recent downstream test run (my mistake for not enabling them on that PR) it appears that downstream libraries are relying on it. If we want to dissuade them from continuing to use it, perhaps we could push a hotfix with non-underscore create_bias marked deprecated? Otherwise we should just restore it and render the docstring, I think.
The tests are also failing due to the rng_from_array() -> default_rng_value() renaming -
config = small: Error During Test at /home/runner/work/Flux.jl/Flux.jl/downstream/test/convnets.jl:332
Got exception outside of a @test
MethodError: no method matching rng_from_array()
Closest candidates are:
rng_from_array(::CUDA.CuArray) at ~/work/Flux.jl/Flux.jl/src/utils.jl:47
rng_from_array(::AbstractArray) at ~/work/Flux.jl/Flux.jl/src/utils.jl:46
Stacktrace:
[1] DropPath(p::Float64)
@ Metalhead.Layers ~/work/Flux.jl/Flux.jl/downstream/src/layers/drop.jl:159
[2] convnextblock(planes::Int64, drop_path_rate::Float64, layerscale_init::Float32)
@ Metalhead ~/work/Flux.jl/Flux.jl/downstream/src/convnets/convnext.jl:14
[3] (::Metalhead.var"#187#188"{Float32, Vector{Int64}, LinRange{Float64, Int64}, Int64})(j::Int64)
@ Metalhead ./none:0
[4] iterate
@ ./generator.jl:47 [inlined]
[5] collect
@ ./array.jl:787 [inlined]
[6] convnext(depths::Vector{Int64}, planes::Vector{Int64}; drop_path_rate::Float64, layerscale_init::Float32, inchannels::Int64, nclasses::Int64)
@ Metalhead ~/work/Flux.jl/Flux.jl/downstream/src/convnets/convnext.jl:60
[7] #ConvNeXt#191
@ ~/work/Flux.jl/Flux.jl/downstream/src/convnets/convnext.jl:99 [inlined]
[8] ConvNeXt
@ ~/work/Flux.jl/Flux.jl/downstream/src/convnets/convnext.jl:97 [inlined]
[9] macro expansion
@ ~/work/Flux.jl/Flux.jl/downstream/test/convnets.jl:333 [inlined]
[10] macro expansion
@ /opt/hostedtoolcache/julia/1.8.0/x64/share/julia/stdlib/v1.8/Test/src/Test.jl:1433 [inlined]
[11] macro expansion
@ ~/work/Flux.jl/Flux.jl/downstream/test/convnets.jl:332 [inlined]
[12] macro expansion
@ /opt/hostedtoolcache/julia/1.8.0/x64/share/julia/stdlib/v1.8/Test/src/Test.jl:1357 [inlined]
[13] top-level scope
@ ~/work/Flux.jl/Flux.jl/downstream/test/convnets.jl:332
[14] include(fname::String)
@ Base.MainInclude ./client.jl:476
[15] macro expansion
@ ~/work/Flux.jl/Flux.jl/downstream/test/runtests.jl:62 [inlined]
[16] macro expansion
@ /opt/hostedtoolcache/julia/1.8.0/x64/share/julia/stdlib/v1.8/Test/src/Test.jl:1357 [inlined]
[17] top-level scope
@ ~/work/Flux.jl/Flux.jl/downstream/test/runtests.jl:62
[18] include(fname::String)
@ Base.MainInclude ./client.jl:476
[19] top-level scope
@ none:6
[20] eval
@ ./boot.jl:368 [inlined]
[21] exec_options(opts::Base.JLOptions)
@ Base ./client.jl:276
[22] _start()
@ Base ./client.jl:522
I think rng_from_array() should be marked as deprecated. For create_bias, I think reverting it and adding it to the manual should be preferred?
Is the default_rng_value() function in a release yet? I can change the Metalhead API and bump compat if it is
default_rng_value() was added in 4773a697f59c1d4dd10e58629f5e8f65a6111c5d, which is not a part of any release. These tests must be failing on the master branch.
Edit: More generally, should the downstream tests run even on the internal API changes (or the ambiguous API changes, used internally but marked as public)?
There's no rule for when downstream tests should run. Some libraries run them unconditionally on every PR, but I think historically they haven't for Flux because of how long some take.
internal API changes (or the ambiguous API changes, used internally but marked as public)?
This is kind of none of the above. There is no real "marked as public" outside of exporting, so downstream libraries were just using a function with a docstring that seemed convenient. Depending on who you ask, the presence/absence of a docstring, leading underscore or even existence of a function are all markers of something being part of the public API...