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

Exported symbol σ too generic #604

Open
nomadbl opened this issue Sep 15, 2024 · 2 comments
Open

Exported symbol σ too generic #604

nomadbl opened this issue Sep 15, 2024 · 2 comments
Labels

Comments

@nomadbl
Copy link

nomadbl commented Sep 15, 2024

Currently NNlib exports σ, which in this package refers to the sigmoid function.
The name should be changed to "sigmoid" to avoid accidental usage in other code. For example the following code will silently use sigmoid (in this example instead of softmax) without raising an error:

julia> using Flux
struct MyOp{F, M<:AbstractMatrix}
  v::M
  σ::F
end
function (a::MyOp)(x::AbstractVecOrMat)
  return σ(a.v * x)
end
a = MyOp(rand32(3,3), Flux.softmax);
x = rand32(3,3);
a(x)
julia> a(x) .- a.σ(a.v*x)
3×3 Matrix{Float32}:
 0.513941  0.363848  0.456001
 0.343419  0.281814  0.289344
 0.519608  0.371748  0.457464
@nomadbl nomadbl changed the title Exported symbol too generic Exported symbol σ too generic Sep 15, 2024
@mcabbott
Copy link
Member

Agree this would be better, although changing it is going to break code.

@ToucheSir
Copy link
Member

FWIW, NNlib already exports sigmoid too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants