-
Notifications
You must be signed in to change notification settings - Fork 3
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
For a 0.2.0 release #30
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Expands existing test set by adding `test_criteria` macro for testing functionality of a Stopping Criterion Detects: #28
This gives a stopping time that directly matches `done!` and avoids issues with not testing `done(::StoppingCriteria, nothing)` Updates tests that are now off due to `stopping_time` previously not stopping until a non-training loss
Previously `update_training(::StoppingCriterion, loss) = nothing`, thus for criteria with only `update` defined, calling `done!(c, loss; training=true)` resulted in a state of `nothing` This builds on that by replacing the following methods: `update(::StoppingCriteria, loss)` -> `update(::StoppingCriteria, loss, ::Nothing)` `update_training(::StoppingCriteria, loss)` -> `update_training(::StoppingCriteria, loss, ::Nothing)` This codifies the idea that `state = nothing` **is** an uninitialized state and eliminates the need for a `isnothing(state)` check in every `update*` method. Also updates all existing criteria to conform to this new api, and adds `isnothing(state)` checks to existing `done` methods and adds check to `test_criteria` to verify `done(criteria, nothing) == false` - Add `isnothing` checks to all `update(::StoppingCriteria, loss, state)` methods -> Rejected as it complicates adding new criteria - Rename all methods to `_update`/ `_update_training` and handle the `isnothing` check with: ```julia function update(criteria, loss, state) if isnothing(state) _update(c, loss) else _update(criteria, loss, state) end end ``` Rejected as it's effectively the same thing without multiple dispatch - Use meta-programming to define `update(criteria, loss, ::Nothing) = false` for all subtypes of `StoppingCriterion` -> Seemed hacky and would inflate precompile times
Using `state = nothing` to indicate an uninitialized criteria, eliminates the need to initialize with `EarlyStopper.state` undefined. This eliminates the need for `done_after*` methods as now both EarlyStopper and the wrapped criteria use `nothing` to indicate an uninitialized state Added `reset!` methods for resetting the state of an `EarlyStopper` - Really just syntactically sugar around setting the stopper's state - Useful when using EarlyStopping.jl to schedule hyperparameters Also fix small typos
Relaxes requirement that training loss is update first and twice between out of sample losses for PQ - Functionally identical to prior code iff a a training strip of length k is followed by a single out-of-sample update - Instead of erroring, PQ waits for the missing samples - Now waits for k training losses (instead of 2) before checking if done (ln 242)
Don't use isnothing (add v1.1) and don't use implict kwargs (Added in v1.5)
Plus some minor edits to the existing docs (Formatting, etc.)
Codify `state == nothing` is an uninitialized state
Codecov Report
@@ Coverage Diff @@
## master #30 +/- ##
==========================================
+ Coverage 98.16% 98.99% +0.82%
==========================================
Files 5 5
Lines 218 199 -19
==========================================
- Hits 214 197 -17
+ Misses 4 2 -2
Continue to review full report at Codecov.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#29