-
-
Notifications
You must be signed in to change notification settings - Fork 24
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
Add all-keyword constructors, much like @kwdef
#160
Conversation
I would really like to see this through. For instance, I hate having to copy and paste the default values of beta1 and beta2 from the docstring of AdamW and write |
Sorry I missed your comment here. But |
No objections from me, though I would put in pin in un-greeking some of the common shared hyperparam names for the next major release. |
prominent direction, in effect dampening oscillations. | ||
- Machine epsilon (`ϵ`): Constant to prevent division by zero | ||
- Machine epsilon (`ϵ == epsilon`): Constant to prevent division by zero | ||
(no need to change default) | ||
- Keyword `centred` (or `centered`): Indicates whether to use centred variant | ||
of the algorithm. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How should these be documented?7706ffd is one suggestion.
We could also remove all greek glyphs from the docstrings, in favour of ascii field names. Some formulae would get a bit longer, or perhaps those few could show both.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe keep the ascii version in the part of the docstring that lists the actual parameter name, then use a latex-ified greek characters in the description of what the parameter does if applicable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure I follow.
Goal of the current style is partly to be usable even if you don't know the names of greek letters. This was true before, as long as you can identify where the same scribble shows up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking of how other libraries use embedded latex in their extended docstrings to describe what optimization rules are doing. e.g. https://pytorch.org/docs/stable/generated/torch.optim.AdamW.html#torch.optim.AdamW. Not something required for every field of every rule, but we don't have to purge greek characters from the docstrings completely.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should just do this.
Docstrings only mention it for a few. Perhaps we can revisit later how best to document this.
Tests fail because Yota is broken, apparently. Perhaps we can replace with Diffractor someday.
@kwdef
* add all-keyword constructors * update a few docstrings * docstrings * add tests * one lost γ should be λ
Allows this:
Not entirely sure it's worth the complexity & extra documentation.
(Note that
adjust!
already exposes field names, so in a sense they are already API.)Checklist