This adds a small metrics struct which stores the information that loss should be minimized, accuracy should be maximized, and generates appropriate file names for checkpoints. It is then used in train.py to implement the checkpointing callback and the patience callback, and in schedulers.py to implement the reduce-on-plateau scheduler.
Whereas #171 used the same flag (--val_metric) to do checkpointing and patience, offline discussion concluded that we want these to be separate. Naturally the reduce-on-plateau scheduler was also left separate.
Some flags have been renamed, and I am happy to discuss this further:
--val_metric is now --checkpoint_metric and --patience_metric.
--reduceonplateau_mode is now --reduceonplateau_metric to match the previous.
I am still bothered by --save_top_k so I have proposed another alternative: --num_checkpoints. Hard to get clearer than that.
Since there has been a decent amount of plumbing changes here, I'd appreciate a closer review. However, my local tests seem just fine.
This PR is a followup to PR #171 and a further response to issue #170.
Just pushed fixes to everything: you want to take a second look before I merge? (Give me the Approve radio button under the review thingy so I can merge without override...)
This adds a small
metrics
struct which stores the information thatloss
should be minimized,accuracy
should be maximized, and generates appropriate file names for checkpoints. It is then used intrain.py
to implement the checkpointing callback and the patience callback, and inschedulers.py
to implement the reduce-on-plateau scheduler.Whereas #171 used the same flag (
--val_metric
) to do checkpointing and patience, offline discussion concluded that we want these to be separate. Naturally the reduce-on-plateau scheduler was also left separate.Some flags have been renamed, and I am happy to discuss this further:
--val_metric
is now--checkpoint_metric
and--patience_metric
.--reduceonplateau_mode
is now--reduceonplateau_metric
to match the previous.--save_top_k
so I have proposed another alternative:--num_checkpoints
. Hard to get clearer than that.Since there has been a decent amount of plumbing changes here, I'd appreciate a closer review. However, my local tests seem just fine.
This PR is a followup to PR #171 and a further response to issue #170.