Skip to content

Conversation

ravi-mosaicml
Copy link
Contributor

Added a run_name as a property of the Logger, and using a coolname to generate it if it is not specified. Moved this logic out of the wandb logger, since it can be useful for every logger. See Artifact Logging for the spec.

CC @abhi-mosaic

1. Add a `run_name` as a property of the Logger, and using a coolname to generate it if it is not specifed. Moved this logic out of the wandb logger.
2. Rename `LoggerCallback` to `LoggerDestination`, and update the hparams and trainer init
3. Removed `will_log`. Having this helper added additional complexity, especially since nothing used deferred logging. Instead, individual callbacks can implement filtering as they see fit.
4. Renamed `log_metric` to `log_data`. Metric is a confusing name, as it can easily be confused with TorchMetrics, and the logger is not limited to TorchMetrics. Similarly, renamed `metric_fit`, `metric_epoch`, and `metric_batch` to `data_fit`, `data_epoch`, and `data_batch`.
5. Removed deep copying of the logging data in the base logger class. It turned out that only the InMemoryLogger needed this functionality, and it copies its own data.
@ravi-mosaicml ravi-mosaicml marked this pull request as ready for review March 9, 2022 17:32
Copy link
Contributor

@ajaysaini725 ajaysaini725 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@jbloxham jbloxham left a comment

Choose a reason for hiding this comment

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

Big fan of this change :)

@ravi-mosaicml ravi-mosaicml added this to the v0.4.2 milestone Mar 14, 2022
@ravi-mosaicml ravi-mosaicml self-assigned this Mar 14, 2022
Base automatically changed from ravi/i421_2 to dev March 15, 2022 01:08
@ravi-mosaicml ravi-mosaicml merged commit 3410bfa into dev Mar 15, 2022
@ravi-mosaicml ravi-mosaicml deleted the ravi/i421_3 branch March 15, 2022 17:42
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