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

Provide aliases for unicode fields and keywords #503

Open
donm opened this issue Nov 21, 2018 · 5 comments
Open

Provide aliases for unicode fields and keywords #503

donm opened this issue Nov 21, 2018 · 5 comments

Comments

@donm
Copy link

donm commented Nov 21, 2018

I'm thinking of adding aliases for keyword arguments and structure fields that currently require using unicode characters to access. Examples include the σ in the Conv field names, and almost all of the fields in BatchNorm:

julia> fieldnames(BatchNorm)
(:λ, :β, :γ, :μ, :σ, :ϵ, :momentum, :active)

I only have three data points so far, but when other people start to use Flux this has been something that bothers them.

Questions:

  • Is this something we should do?
  • Would you prefer having ascii names added as aliases, or replacing the current instances of unicode with ascii names and then adding the current unicode names back as aliases?
  • Should the ascii name simply spell out the unicode symbol, or describe meaning of the field/variable? For example, should the alias for σ in Conv be activation or sigma? (My preference would be activation.)
@MikeInnes
Copy link
Member

I generally agree that anything user-facing should not have unicode in it (and there are a few places we need to fix this). At the same time fields like this are generally considered private, and it's better form to provide accessor functions (e.g. Flux.activation(layer)).

On the flip side of that, it's probably not that reasonable to have an accessor function for every possible component of a layer, especially for things like batchnorm. If people want to access those fields (is this common?) then it'd make sense to rename them clearly as you suggest.

Open to discussion on this.

@donm
Copy link
Author

donm commented Dec 1, 2018

When you say user-facing interfaces should not have unicode, do you mean they shouldn't force the use of unicode, or shouldn't use it at all?

I'm not sure if accessing those types of fields is common, but it's definitely common for me and others I work with. Being able to reach in and quickly manipulate weights and other state in the middle of training is a huge benefit of having the whole library written in Julia.

I agree that the number of accessor functions could be unwieldy, especially because many of the fields would need both getters and setters.

@MikeInnes
Copy link
Member

Right, no one should be forced to use unicode to build a model with Flux -- which sounds like it's the case for you.

We can change the fieldnames but should also add a deprecation for the current names. I'm open to PRs on this.

@namanbiyani
Copy link

namanbiyani commented Mar 3, 2019

@MikeInnes I would like to work on this . please tell which field names should be changed in which files are they present?

@MikeInnes
Copy link
Member

If you want to start with batchnorm, that's in src/layers/normalise.jl.

namanbiyani added a commit to namanbiyani/Flux.jl that referenced this issue Mar 4, 2019
FluxML#503 Provided aliases for unicode fields in batchnorm and dropout
namanbiyani added a commit to namanbiyani/Flux.jl that referenced this issue Mar 4, 2019
FluxML#503 Provided aliases for unicode fields in batchnorm and dropout
namanbiyani added a commit to namanbiyani/Flux.jl that referenced this issue Mar 4, 2019
FluxML#503 Provided aliases for unicode fields in batchnorm and dropout
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants