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.

Sign up for GitHub

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

Jump to bottom

Use tensor_flow_addons #170

Merged
merged 15 commits into from
Mar 23, 2020
Merged

Use tensor_flow_addons #170

merged 15 commits into from
Mar 23, 2020

Conversation

ndiamant
Copy link
Contributor

  • learning rate finding, and find then train recipes added
  • learning rate schedules added (triangular, triangular2)
  • RAdam dependency removed
  • lookahead removed (could be easily readded with tfa)
  • More normalizations added, though inaccessible from the command line now, because untested

@ndiamant ndiamant mentioned this pull request Mar 16, 2020
Closed
lucidtronix
lucidtronix requested changes Mar 16, 2020
View reviewed changes
Copy link
Collaborator

@lucidtronix lucidtronix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! Some small nitpicks inline. Bigger question is we still use ReduceLROnPlateau by default during training, does that still make sense with the LR finding done here? while we're at it should we make that command line toggle-able

ml4cvd/recipes.py Outdated Show resolved Hide resolved
ml4cvd/recipes.py Outdated Show resolved Hide resolved
ml4cvd/optimizers.py Outdated Show resolved Hide resolved
ml4cvd/arguments.py Show resolved Hide resolved
@ndiamant
Copy link
Contributor Author

ndiamant commented Mar 16, 2020 •
edited
Loading

Bigger question is we still use ReduceLROnPlateau by default during training, does that still make sense with the LR finding done here? while we're at it should we make that command line toggle-able

@lucidtronix ReduceLROnPlateau along with the other callbacks should be more flexibly accessible I think. For example, I left off adding SWA, or the ensemble from triangle cycles in this PR partly because it would take bigger changes to add them as callbacks. For now, if you use a learning rate schedule, you should set the patience higher than the number of epochs so that that callback does not get used.

@ndiamant
Copy link
Contributor Author

Reminder to push docker with tfa before merge

@lucidtronix lucidtronix self-requested a review March 20, 2020 19:51
lucidtronix
lucidtronix approved these changes Mar 20, 2020
View reviewed changes
Copy link
Collaborator

@lucidtronix lucidtronix left a comment •
edited by ndiamant
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! If tests pass push docker (GPU & CPU) images and merge at will!

Tests pass!

@ndiamant ndiamant merged commit a513ef4 into master Mar 23, 2020
@ndiamant ndiamant deleted the nd_tfa branch March 23, 2020 15:20
lucidtronix pushed a commit that referenced this pull request Jan 13, 2023
@ndiamant
Use tensor_flow_addons (#170)
1e8c1aa 
* rectified adam

* removed lookahead, because it is unused and could be implemented with tfa

* added more activation options

* more normalization schemes, learning rate schedules

* added learning rate finder

* bug fixes

* delta for best lr picking in its own function

* smoothing finds better lrs

* small fix

* plot_metrics handles learning rate schedulers

* learning rate schedule works with _find_lr

* reorganization

* bug fix

Co-authored-by: ndiamant <ndiamant@broadinstitute.org>
lucidtronix pushed a commit that referenced this pull request Jan 13, 2023
@ndiamant
Use tensor_flow_addons (#170)
4c3fd44 
* rectified adam

* removed lookahead, because it is unused and could be implemented with tfa

* added more activation options

* more normalization schemes, learning rate schedules

* added learning rate finder

* bug fixes

* delta for best lr picking in its own function

* smoothing finds better lrs

* small fix

* plot_metrics handles learning rate schedulers

* learning rate schedule works with _find_lr

* reorganization

* bug fix

Co-authored-by: ndiamant <ndiamant@broadinstitute.org>
lucidtronix pushed a commit that referenced this pull request Jan 13, 2023
@ndiamant
Use tensor_flow_addons (#170)
15e275c 
* rectified adam

* removed lookahead, because it is unused and could be implemented with tfa

* added more activation options

* more normalization schemes, learning rate schedules

* added learning rate finder

* bug fixes

* delta for best lr picking in its own function

* smoothing finds better lrs

* small fix

* plot_metrics handles learning rate schedulers

* learning rate schedule works with _find_lr

* reorganization

* bug fix

Co-authored-by: ndiamant <ndiamant@broadinstitute.org>
lucidtronix pushed a commit that referenced this pull request Jan 13, 2023
@ndiamant
Use tensor_flow_addons (#170)
3b71e31 
* rectified adam

* removed lookahead, because it is unused and could be implemented with tfa

* added more activation options

* more normalization schemes, learning rate schedules

* added learning rate finder

* bug fixes

* delta for best lr picking in its own function

* smoothing finds better lrs

* small fix

* plot_metrics handles learning rate schedulers

* learning rate schedule works with _find_lr

* reorganization

* bug fix

Co-authored-by: ndiamant <ndiamant@broadinstitute.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lucidtronix lucidtronix lucidtronix approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ndiamant @lucidtronix