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.

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
merged 9 commits into from
Oct 27, 2021
Merged

For a 0.2.0 release #30

merged 9 commits into from
Oct 27, 2021

Conversation

ablaom
Copy link
Member

@ablaom ablaom commented Oct 27, 2021

#29

awadell1 and others added 9 commits October 22, 2021 12:41
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-commenter
Copy link

codecov-commenter commented Oct 27, 2021

Codecov Report

Merging #30 (c4cfe9e) into master (3acfbbb) will increase coverage by 0.82%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            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     
Impacted Files Coverage Δ
src/api.jl 100.00% <100.00%> (ø)
src/criteria.jl 98.50% <100.00%> (+1.42%) ⬆️
src/disjunction.jl 100.00% <100.00%> (ø)
src/object_oriented_api.jl 100.00% <100.00%> (ø)
src/stopping_time.jl 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3acfbbb...c4cfe9e. Read the comment docs.

@ablaom ablaom merged commit b8e981b into master Oct 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants