Use consistent spelling for optimise #2203
Conversation
Codecov ReportPatch coverage has no change and project coverage change:
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## master #2203 +/- ##
==========================================
- Coverage 82.63% 79.93% -2.70%
==========================================
Files 23 23
Lines 1578 1575 -3
==========================================
- Hits 1304 1259 -45
- Misses 274 316 +42
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
The problem is that the majority of current maintainers use Z, but the original code (including the unfortunately named Optimisers.jl) was written by someone who uses S. I guess this is as good a chance as any to debate which one we should use going forward. |
|
I'm in favor of "optimiz*" everywhere. |
There was a problem hiding this comment.
This seems OK, it's a minor spelling fix to docs. There is no way that the docs are ever likely to be fully consistent about using UK spelling, although I do think that's the convention.
Trying to avoid code which requires one spelling seems like a good prinicple. E.g. we should kill Flux.normalise. (Edit: JuliaML/MLUtils.jl#123 is the issue.)
Am I correct in thinking that the only place where Flux's code cares about the spelling of "optimiser" is the package name Optimisers.jl ? Renaming that would be a much bigger step. Perhaps one we should have contemplated before registering it -- I mean to some other name completely, both to avoid words whose spelling varies, and to avoid the (temporary) clash with the Flux.Optimise sub-module.
| gradients = gradient(() -> loss(x, y), parameters) | ||
| Flux.Optimise.update!(optimizer, parameters, gradients) | ||
| Flux.Optimise.update!(optimiser, parameters, gradients) |
There was a problem hiding this comment.
This ought to be re-written in gradients parameters anyway, in which case the variable will be opt_state
|
|
||
| You can store the optimiser state alongside the model, to resume training | ||
| exactly where you left off. BSON is smart enough to [cache values](https://github.com/JuliaIO/BSON.jl/blob/v0.3.4/src/write.jl#L71) and insert links when saving, but only if it knows everything to be saved up front. Thus models and optimizers must be saved together to have the latter work after restoring. | ||
| exactly where you left off. BSON is smart enough to [cache values](https://github.com/JuliaIO/BSON.jl/blob/v0.3.4/src/write.jl#L71) and insert links when saving, but only if it knows everything to be saved up front. Thus models and optimisers must be saved together to have the latter work after restoring. |
There was a problem hiding this comment.
To be re-written for explicit gradients
| @@ -145,7 +145,7 @@ function train(; kws...) | |||
| return logitcrossentropy(ŷ, y) | |||
| end | |||
|
|
|||
| # Train our model with the given training set using the Adam optimizer and | |||
| # Train our model with the given training set using the Adam optimiser and | |||
There was a problem hiding this comment.
This file to be deleted in favour of the model zoo, perhaps FluxML/model-zoo#394
| @@ -45,7 +45,7 @@ end | |||
| """ | |||
| Momentum(η = 0.01, ρ = 0.9) | |||
|
|
|||
| Gradient descent optimizer with learning rate `η` and momentum `ρ`. | |||
| Gradient descent optimiser with learning rate `η` and momentum `ρ`. | |||
There was a problem hiding this comment.
This file will be deleted in 0.14
Flux.jl uses alternately optimize and optimise, in various parts of the project, which can be a bit tricky, particularly as the JuMP.jl ecosystem seems to have chosen optimize. Not sure whether it would make sense to support both spellings, but in the meantime it seems best to be internally consistent.
PR Checklist