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

Load ckpt path when model provided in validate/test/predict #8352

Merged
merged 40 commits into from
Jul 28, 2021
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
03a8769
Change trainer loading behaviour for validate/test/predict
Jul 9, 2021
a943e33
Fix
Jul 9, 2021
40a3446
Fix/add tests
Jul 9, 2021
8c24ffd
remove
Jul 9, 2021
1879be7
Cleanups
Jul 12, 2021
3162ff7
Space
Jul 12, 2021
6dd61d6
cleanups
Jul 12, 2021
5772e17
Merge branch 'master' into feat/ckpt_load
Jul 12, 2021
b072868
Add CHANGELOG.md
Jul 12, 2021
de2738d
Merge branch 'master' into feat/ckpt_load
Jul 12, 2021
f2ee8b5
Move after setup
Jul 12, 2021
8659426
Cleanups on logic
Jul 12, 2021
84d20f5
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jul 12, 2021
9e367fd
Remve
Jul 12, 2021
b8ffc39
fix test
Jul 12, 2021
b02f35b
feedback
Jul 12, 2021
dbb03af
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jul 12, 2021
1c7b9a1
Update pytorch_lightning/trainer/properties.py
Jul 12, 2021
444fb55
Feedback
Jul 12, 2021
4632bba
Same fix
Jul 12, 2021
e92b757
Same fix
Jul 12, 2021
66bea8e
Add test for behaviour, modify based on feedback
Jul 12, 2021
0139a19
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jul 12, 2021
d48d916
Wording
Jul 12, 2021
100d73b
Apply suggestions from code review
Jul 12, 2021
f3f92a5
Cleanup docs
Jul 12, 2021
2849d0b
Update pytorch_lightning/trainer/trainer.py
Jul 12, 2021
f53c896
feedback
Jul 12, 2021
ebc713b
Fixes to test API
Jul 12, 2021
76e22c2
Add carlos description
Jul 12, 2021
9c2a0de
Move logic further
Jul 13, 2021
e5c104c
Merge branch 'master' into feat/ckpt_load
Jul 13, 2021
be07eec
Move checkpoint connector logic
Jul 13, 2021
a9538d6
revert
Jul 13, 2021
e5a0dba
Remove 4 as this is a dupe now
Jul 27, 2021
5136b61
Merge branch 'master' into feat/ckpt_load
Jul 27, 2021
9867b38
fix
Jul 27, 2021
3c16048
set to best
Jul 27, 2021
b2ba6db
Fix location
Jul 28, 2021
33f8917
Merge branch 'master' into feat/ckpt_load
Jul 28, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,9 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
- Added support for `accelerator='cpu'|'gpu'|'tpu'|'ipu'|'auto'` ([#7808](https://github.com/PyTorchLightning/pytorch-lightning/pull/7808))


- Load ckpt path when model provided in validate/test/predict ([#8352](https://github.com/PyTorchLightning/pytorch-lightning/pull/8352)))


### Changed


Expand Down
14 changes: 14 additions & 0 deletions pytorch_lightning/trainer/properties.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,11 @@ class TrainerProperties(ABC):
validate_loop: EvaluationLoop
test_loop: EvaluationLoop
predict_loop: PredictionLoop

# .validate() and .test() set this when they load a checkpoint
validated_ckpt_path: Optional[str] = None
tested_ckpt_path: Optional[str] = None
predicted_ckpt_path: Optional[str] = None
"""
Accelerator properties
"""
Expand Down Expand Up @@ -548,6 +553,15 @@ def _active_loop(self) -> Optional[Union[FitLoop, EvaluationLoop, PredictionLoop
if self.predicting:
return self.predict_loop

@property
def _ckpt_path(self) -> Optional[str]:
if self.state.fn == TrainerFn.VALIDATING:
return self.validated_ckpt_path
if self.state.fn == TrainerFn.TESTING:
return self.tested_ckpt_path
if self.state.fn == TrainerFn.PREDICTING:
return self.predicted_ckpt_path

"""
Logging properties
"""
Expand Down
61 changes: 35 additions & 26 deletions pytorch_lightning/trainer/trainer.py
Original file line number Diff line number Diff line change
Expand Up @@ -458,15 +458,10 @@ def _setup_on_init(
self.test_dataloaders = None
self.val_dataloaders = None

# .validate() and .test() set this when they load a checkpoint
self.validated_ckpt_path = None
self.tested_ckpt_path = None

# when true, print evaluation results in .validate() and .test()
self.verbose_evaluate = True

self.num_predict_batches = []
self.predicted_ckpt_path = None

def fit(
self,
Expand Down Expand Up @@ -544,7 +539,7 @@ def validate(

ckpt_path: Either ``best`` or path to the checkpoint you wish to validate.
If ``None``, use the current weights of the model.
When the model is given as argument, this parameter will not apply.
When the model is given as argument, we load the ckpt path.
SeanNaren marked this conversation as resolved.
Show resolved Hide resolved

verbose: If True, prints the validation results.

Expand Down Expand Up @@ -589,8 +584,9 @@ def validate(
# links data to the trainer
self.data_connector.attach_data(model, val_dataloaders=dataloaders, datamodule=datamodule)

if not model_provided:
self.validated_ckpt_path = self.__load_ckpt_weights(ckpt_path)
self.validated_ckpt_path = self.__set_ckpt_path(
ckpt_path, model_provided=model_provided, model_connected=self.lightning_module is not None
)

# run validate
results = self._run(model)
Expand Down Expand Up @@ -621,7 +617,7 @@ def test(

ckpt_path: Either ``best`` or path to the checkpoint you wish to test.
If ``None``, use the current weights of the model.
When the model is given as argument, this parameter will not apply.
When the model is given as argument, we load the ckpt path.

verbose: If True, prints the test results.

Expand Down Expand Up @@ -664,8 +660,9 @@ def test(
# links data to the trainer
self.data_connector.attach_data(model, test_dataloaders=dataloaders, datamodule=datamodule)

if not model_provided:
self.tested_ckpt_path = self.__load_ckpt_weights(ckpt_path)
self.tested_ckpt_path = self.__set_ckpt_path(
ckpt_path, model_provided=model_provided, model_connected=self.lightning_module is not None
)

# run test
results = self._run(model)
Expand Down Expand Up @@ -699,9 +696,9 @@ def predict(
return_predictions: Whether to return predictions.
``True`` by default except when an accelerator that spawns processes is used (not supported).

ckpt_path: Either ``best`` or path to the checkpoint you wish to use to predict.
ckpt_path: Either ``best`` or path to the checkpoint you wish to predict.
If ``None``, use the current weights of the model.
When the model is given as argument, this parameter will not apply.
When the model is given as argument, we load the ckpt path.

Returns:
Returns a list of dictionaries, one for each provided dataloader containing their respective predictions.
Expand Down Expand Up @@ -735,8 +732,9 @@ def predict(
# links data to the trainer
self.data_connector.attach_data(model, predict_dataloaders=dataloaders, datamodule=datamodule)

if not model_provided:
self.predicted_ckpt_path = self.__load_ckpt_weights(ckpt_path)
self.predicted_ckpt_path = self.__set_ckpt_path(
ckpt_path, model_provided=model_provided, model_connected=self.lightning_module is not None
)

results = self._run(model)

Expand Down Expand Up @@ -838,6 +836,15 @@ def _run(self, model: 'pl.LightningModule') -> Optional[Union[_EVALUATE_OUTPUT,
self._call_configure_sharded_model(model) # allow user to setup in model sharded environment
self.accelerator.setup(self, model) # note: this sets up self.lightning_module

if self._ckpt_path:
# only one process running at this point for TPUs, as spawn isn't triggered yet
# todo: move this logic internally within the barrier.
if not self._device_type == DeviceType.TPU:
self.training_type_plugin.barrier()

rank_zero_info(f"Loading checkpoint from {self._ckpt_path}")
self.checkpoint_connector.restore_model_weights(self._ckpt_path)

# ----------------------------
# INSPECT THE CORE LOOPS
# ----------------------------
Expand Down Expand Up @@ -1072,13 +1079,22 @@ def _run_sanity_check(self, ref_model):
# restore the previous stage when the sanity check if finished
self.state.stage = stage

def __load_ckpt_weights(self, ckpt_path: Optional[str]) -> Optional[str]:
if ckpt_path is None:
return
def __set_ckpt_path(self, ckpt_path: Optional[str], model_provided: bool, model_connected: bool) -> Optional[str]:
if model_provided and ckpt_path is None:
return # use passed model to function without loading weights

fn = self.state.fn.value

if ckpt_path == 'best':
if model_connected and ckpt_path is None:
rank_zero_warn(
f"`.{fn}(ckpt_path=None)` was called without a model. "
f"The best model of the previous `fit` call will be used. "
f"You can pass `ckpt_path='best'` to avoid this warning "
f"or `ckpt_path=trainer.model_checkpoint.last_model_path` to use the last model."
SeanNaren marked this conversation as resolved.
Show resolved Hide resolved
)
ckpt_path = 'best'

if (model_connected or model_provided) and ckpt_path == 'best':
SeanNaren marked this conversation as resolved.
Show resolved Hide resolved
# if user requests the best checkpoint but we don't have it, error
if not self.checkpoint_callback.best_model_path:
if self.fast_dev_run:
Expand All @@ -1097,13 +1113,6 @@ def __load_ckpt_weights(self, ckpt_path: Optional[str]) -> Optional[str]:
f'`.{fn}()` found no path for the best weights: "{ckpt_path}". Please'
f' specify a path for a checkpoint `.{fn}(ckpt_path=PATH)`'
)

# only one process running at this point for TPUs, as spawn isn't triggered yet
# todo: move this logic internally within the barrier.
if not self._device_type == DeviceType.TPU:
self.training_type_plugin.barrier()

self.checkpoint_connector.restore_model_weights(ckpt_path)
return ckpt_path

def _call_setup_hook(self, model: 'pl.LightningModule') -> None:
Expand Down
4 changes: 4 additions & 0 deletions tests/trainer/test_dataloaders.py
Original file line number Diff line number Diff line change
Expand Up @@ -740,6 +740,8 @@ def test_warning_with_few_workers(_, tmpdir, ckpt_path, stage):
match=f'The dataloader, {stage} dataloader{" 0" if stage != "train" else ""}, does not have many workers'
):
if stage == 'test':
if ckpt_path == 'specific':
trainer.fit(model, train_dataloader=train_dl, val_dataloaders=val_dl)
ckpt_path = trainer.checkpoint_callback.best_model_path if ckpt_path == 'specific' else ckpt_path
trainer.test(model, test_dataloaders=train_dl, ckpt_path=ckpt_path)
else:
Expand Down Expand Up @@ -782,6 +784,8 @@ def test_warning_with_few_workers_multi_loader(_, tmpdir, ckpt_path, stage):
match=f'The dataloader, {stage} dataloader{" 0" if stage != "train" else ""}, does not have many workers'
):
if stage == 'test':
if ckpt_path == 'specific':
trainer.fit(model, train_dataloader=train_multi_dl, val_dataloaders=val_multi_dl)
SeanNaren marked this conversation as resolved.
Show resolved Hide resolved
ckpt_path = trainer.checkpoint_callback.best_model_path if ckpt_path == 'specific' else ckpt_path
trainer.test(model, test_dataloaders=test_multi_dl, ckpt_path=ckpt_path)
else:
Expand Down
19 changes: 16 additions & 3 deletions tests/trainer/test_trainer.py
Original file line number Diff line number Diff line change
Expand Up @@ -680,14 +680,24 @@ def predict_step(self, batch, *_):
if save_top_k == 0:
with pytest.raises(MisconfigurationException, match=".*is not configured to save the best.*"):
trainer_fn(ckpt_path=ckpt_path)
with pytest.raises(MisconfigurationException, match=".*is not configured to save the best.*"):
trainer_fn(model, ckpt_path=ckpt_path)
else:
trainer_fn(ckpt_path=ckpt_path)
assert getattr(trainer, path_attr) == trainer.checkpoint_callback.best_model_path

trainer_fn(model, ckpt_path=ckpt_path)
assert getattr(trainer, path_attr) == trainer.checkpoint_callback.best_model_path
elif ckpt_path is None:
# ckpt_path is None, meaning we don't load any checkpoints and
# use the weights from the end of training
trainer_fn(ckpt_path=ckpt_path)
# ckpt_path is None, meaning we don't load any checkpoints and use the provided model
trainer_fn(model, ckpt_path=ckpt_path)
assert getattr(trainer, path_attr) is None

if save_top_k > 0:
# ckpt_path is None with no model provided means load the best weights
with pytest.warns(UserWarning, match="The best model of the previous `fit` call will be used"):
trainer_fn(ckpt_path=ckpt_path)
assert getattr(trainer, path_attr) == trainer.checkpoint_callback.best_model_path
else:
# specific checkpoint, pick one from saved ones
if save_top_k == 0:
Expand All @@ -701,6 +711,9 @@ def predict_step(self, batch, *_):
trainer_fn(ckpt_path=ckpt_path)
assert getattr(trainer, path_attr) == ckpt_path

trainer_fn(model, ckpt_path=ckpt_path)
assert getattr(trainer, path_attr) == ckpt_path


def test_disabled_training(tmpdir):
"""Verify that `limit_train_batches=0` disables the training loop unless `fast_dev_run=True`."""
Expand Down