FluxML / Flux.jl

Relax! Flux is the ML library that doesn't make you tensor
https://fluxml.ai/
Other
4.53k stars 610 forks source link

Provide aliases for unicode fields and keywords #503

Open donm opened 5 years ago

donm commented 5 years ago

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:

MikeInnes commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

MikeInnes commented 5 years ago

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