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

Create loggers property for Trainer and LightningModule #11683

Merged
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
63 commits
Select commit Hold shift + click to select a range
2883fb2
Implement logger property, replace self.logger
akashkw Feb 1, 2022
3ae0a6e
Fix small bugs
akashkw Feb 1, 2022
6d917a4
Fixed initalization bug
akashkw Feb 1, 2022
30d3a57
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Feb 1, 2022
62c9335
Fix for case where creating a LoggerCollection of size 1
akashkw Feb 1, 2022
99a4d98
Better fix for the LoggerCollection of size 1 issue
akashkw Feb 1, 2022
01287ba
Change trainer.loggers from a property to an instance variable
akashkw Feb 1, 2022
41cbab0
Revert all instances of trainer.loggers being used
akashkw Feb 1, 2022
d78d98f
Use logger param to initialize trainer.loggers
akashkw Feb 1, 2022
7f809ff
Remove unneeded newlines
akashkw Feb 1, 2022
179f3d4
Implement unit test for loggers property
akashkw Feb 1, 2022
1ad542a
make trainer.loggers by default an empty list
akashkw Feb 1, 2022
c5df0ad
Update changelog
akashkw Feb 1, 2022
9d564d8
fix unit test according to suggestions
akashkw Feb 1, 2022
9d7c1bf
Update CHANGELOG.md
akashkw Feb 1, 2022
d263659
Remove unnecessary Trainer params
akashkw Feb 2, 2022
befad11
Remove tmpdir parameter for unit test
akashkw Feb 2, 2022
4773c26
Write setters for logger and loggers
akashkw Feb 2, 2022
8871c36
Unit test for setters
akashkw Feb 2, 2022
65ae649
Fix bug where logger setter is called twice
akashkw Feb 2, 2022
df992de
Fix initialization bug with trainer test
akashkw Feb 2, 2022
5e47ef4
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Feb 2, 2022
d55f626
Get rid of extra DummyLogger assignment
akashkw Feb 2, 2022
3debee7
Merge branch 'refactor/create-loggers-property' of github.com:akashkw…
akashkw Feb 2, 2022
29778b1
flake and mypy fixes
akashkw Feb 2, 2022
506e5fd
Flake fix did not commit properly
akashkw Feb 2, 2022
144169b
Small changes based on suggestions
akashkw Feb 2, 2022
bdcbcfb
Shorten setters and update unit test
akashkw Feb 2, 2022
d42ce90
Move unit test to a new file
akashkw Feb 2, 2022
4001efc
flake and mypy fixes
akashkw Feb 2, 2022
9401fb7
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Feb 2, 2022
e2be787
Refactor setter to handle special case of size 1 LoggerCollection
akashkw Feb 3, 2022
1e71c40
Remove DummyLogger changes
akashkw Feb 3, 2022
4418f10
Merge branch 'master' into refactor/create-loggers-property
akashkw Feb 3, 2022
c96534b
Commit suggestion to change None to []
akashkw Feb 3, 2022
6a6e2ef
Merge branch 'master' into refactor/create-loggers-property
akashkw Feb 3, 2022
7fef6fd
Decouple both setters for readability
akashkw Feb 3, 2022
f2a598f
Fix merge conflicts
akashkw Feb 3, 2022
a669a98
Fix tiny bug in trainer.loggers setter
akashkw Feb 3, 2022
b6d6fba
Merge branch 'master' into refactor/create-loggers-property
akashkw Feb 3, 2022
8714857
Add loggers property to lightning.py
akashkw Feb 3, 2022
a7a47f1
update changelog
akashkw Feb 3, 2022
15fa585
Add logger property to docs
akashkw Feb 3, 2022
3f195be
Fix typo
akashkw Feb 3, 2022
c944f9a
update trainer.rst
akashkw Feb 3, 2022
e3312d5
correct spacing
akashkw Feb 3, 2022
88b3c24
remove typing for now
akashkw Feb 3, 2022
89f5037
Merge branch 'master' into refactor/create-loggers-property
akashkw Feb 3, 2022
85256e7
Fix jit unused issue
akashkw Feb 3, 2022
7ab3683
Fix underlines in docs
akashkw Feb 3, 2022
c51f336
Updates based on suggestions
akashkw Feb 4, 2022
ec99acd
More updates to docs based on suggestions
akashkw Feb 4, 2022
ba09e27
Create unit test for lightningmodule loggers property
akashkw Feb 4, 2022
bc6fd72
Replace Mock with Trainer
akashkw Feb 4, 2022
f5b492d
Update types
akashkw Feb 4, 2022
8800ec8
Remove list cast
akashkw Feb 4, 2022
5852ee2
Remove unit test for unsupported behavior
akashkw Feb 4, 2022
0752fb5
Handle special case for setter
akashkw Feb 4, 2022
46b5ae1
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Feb 4, 2022
3480c38
Update docs/source/common/lightning_module.rst
akashkw Feb 9, 2022
cd90c34
Refactor docs and trainer according to suggestions
akashkw Feb 9, 2022
cd43e25
Resolve merge conflicts
akashkw Feb 9, 2022
338ba43
Update unit tests with new behavior
akashkw Feb 9, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -78,17 +78,21 @@ def should_update_logs(self) -> bool:
def configure_logger(self, logger: Union[bool, LightningLoggerBase, Iterable[LightningLoggerBase]]) -> None:
if isinstance(logger, bool):
# default logger
self.trainer.logger = (
TensorBoardLogger(
if logger:
default_logger = TensorBoardLogger(
save_dir=self.trainer.default_root_dir, version=SLURMEnvironment.job_id(), name="lightning_logs"
)
if logger
else None
)
self.trainer.logger = default_logger
self.trainer.loggers = [default_logger]
else:
self.trainer.logger = None
self.trainer.loggers = []
elif isinstance(logger, Iterable):
self.trainer.logger = LoggerCollection(logger)
self.trainer.loggers = list(logger)
else:
self.trainer.logger = logger
self.trainer.loggers = [logger]

def log_metrics(self, metrics: _OUT_DICT, step: Optional[int] = None) -> None:
"""Logs the metric dict passed in. If `step` parameter is None and `step` key is presented is metrics, uses
Expand Down
5 changes: 4 additions & 1 deletion pytorch_lightning/trainer/trainer.py
Expand Up @@ -565,6 +565,7 @@ def __init__(

# init logger flags
self.logger: Optional[LightningLoggerBase]
self.loggers: List[LightningLoggerBase]
self.logger_connector.on_trainer_init(logger, flush_logs_every_n_steps, log_every_n_steps, move_metrics_to_cpu)

# init debugging flags
Expand Down Expand Up @@ -613,7 +614,9 @@ def _init_debugging_flags(
self.fit_loop.max_epochs = 1
val_check_interval = 1.0
self.check_val_every_n_epoch = 1
self.logger = DummyLogger() if self.logger is not None else None
if self.logger is not None:
self.logger = DummyLogger()
self.loggers = [self.logger]

rank_zero_info(
"Running in fast_dev_run mode: will run a full train,"
Expand Down
26 changes: 26 additions & 0 deletions tests/loggers/test_base.py
Expand Up @@ -176,6 +176,32 @@ def training_step(self, batch, batch_idx):
assert logger2.finalized_status == "success"


def test_loggers_property(tmpdir):
akashkw marked this conversation as resolved.
Show resolved Hide resolved
logger1 = CustomLogger()
logger2 = CustomLogger()

# trainer.loggers should be a copy of the input list
trainer = Trainer(max_steps=2, log_every_n_steps=1, logger=[logger1, logger2], default_root_dir=tmpdir)
akashkw marked this conversation as resolved.
Show resolved Hide resolved

assert trainer.loggers == [logger1, logger2]

# trainer.loggers should create a list of size 1
trainer = Trainer(max_steps=2, log_every_n_steps=1, logger=logger1, default_root_dir=tmpdir)

assert trainer.loggers == [logger1]

# trainer.loggers should be None
akashkw marked this conversation as resolved.
Show resolved Hide resolved
trainer = Trainer(max_steps=2, log_every_n_steps=1, logger=False, default_root_dir=tmpdir)

assert trainer.loggers is []

# trainer.loggers should be a list of size 1 holding the default logger
trainer = Trainer(max_steps=2, log_every_n_steps=1, logger=True, default_root_dir=tmpdir)

assert trainer.loggers == [trainer.logger]
assert type(trainer.loggers[0]) == TensorBoardLogger


def test_multiple_loggers_pickle(tmpdir):
"""Verify that pickling trainer with multiple loggers works."""

Expand Down