Added metrics logging to checkpoint and separate yaml file#1562
Merged
BloodAxe merged 24 commits intoOct 31, 2023
Conversation
philmarchenko
force-pushed
the
feature/saving_metrics_to_yaml
branch
from
October 23, 2023 11:01
6b3999c to
fb8ae7d
Compare
BloodAxe
reviewed
Oct 23, 2023
BloodAxe
reviewed
Oct 23, 2023
BloodAxe
reviewed
Oct 23, 2023
BloodAxe
reviewed
Oct 23, 2023
philmarchenko
marked this pull request as ready for review
October 24, 2023 17:54
philmarchenko
requested review from
Louis-Dupont,
ofrimasad and
shaydeci
as code owners
October 24, 2023 17:54
BloodAxe
reviewed
Oct 25, 2023
BloodAxe
reviewed
Oct 25, 2023
BloodAxe
reviewed
Oct 25, 2023
…6/super-gradients into feature/saving_metrics_to_yaml
Contributor
Author
|
After a discussion with @BloodAxe we decided to leave only saving metrics to a checkpoint for this pull request. Another pull request will come with proposals for sg_loggers refactoring and saving some trainer internals to yaml. |
BloodAxe
reviewed
Oct 26, 2023
BloodAxe
reviewed
Oct 26, 2023
BloodAxe
reviewed
Oct 26, 2023
This file contains hidden or 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
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.
Issue description
@BloodAxe said SG does not support saving metrics, environment, and train config to the checkpoint.
After getting more familiar with a codebase, I realized the task should be split into several parts / PRs.
PR description
This PR proposes a way to add metrics to a checkpoint.
Primarily, this is done for easier access to experiment metrics, so the user doesn't need to run TB or parse the checkpoint to access them.
Some ideas and notes
Let's suppose the user needs to train a custom model with several classification heads and track different metrics. For now, all the metrics are gathered in a MetricCollection and each metric uses the same output of a model. Moreover, if the user wants to track the accuracy of some of the outputs, he can't do this. Now metrics are named exactly as the class is called and are stored in the dictionary under this name. It will be rewritten into the dictionary and one metric will remain.
It would be great to have the possibility to specify which model output should be used for every metric (and maybe loss). It also could be configured somehow like
When someone tries to access metric/loss values separately after executing the evaluate() method, some routine on splitting losses and metrics from single joined dict should be done. In this PR, saving metrics to the dict could be done much easier, if they were separated (for now the metrics are selected from the dictionary by their names and form a new dictionary, see sg_trainer.py/_save_checkpoint/lines 671, 679). Also, treating losses and metrics as separate structures adds some points to the code understanding, as they are logically different ;)
It looks like changing this will affect a bunch of places in the project, so if code owners respond to this problem, then it should become a separate PR :)
There are some places in _save_checkpoint function where a type of metric value should be checked wrt its type (values returned in validation/train_results_dict). It looks like metrics there sometimes appear to have different types (torch.Tensor(dtype=float) or a usual float). It would be much more convenient to expect a single type of value there, and checks of this kind will disappear and make the code look cleaner 🪄
For now, this routine has been moved to a separate function.