Add trainer.predict(ckpt_path)
#7430
Conversation
carmocca
requested review from
awaelchli,
Borda,
justusschock,
kaushikb11,
SeanNaren,
tchaton and
williamFalcon
as code owners
Codecov Report
@@ Coverage Diff @@
## master #7430 +/- ##
======================================
- Coverage 92% 92% -0%
======================================
Files 199 199
Lines 13067 13072 +5
======================================
- Hits 11997 11990 -7
- Misses 1070 1082 +12 |
| # links data to the trainer | ||
| self.data_connector.attach_data(model, predict_dataloaders=dataloaders, datamodule=datamodule) | ||
|
|
||
| if not model_provided: |
There was a problem hiding this comment.
use model directly. No need for model_provided.
There was a problem hiding this comment.
Why not? We use it also in validate and test
There was a problem hiding this comment.
@tchaton it canot as you do the mapping after it... I quess
| # links data to the trainer | ||
| self.data_connector.attach_data(model, predict_dataloaders=dataloaders, datamodule=datamodule) | ||
|
|
||
| if not model_provided: |
There was a problem hiding this comment.
@tchaton it canot as you do the mapping after it... I quess
Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com>
| trainer_fn = getattr(trainer, fn) | ||
| path_attr = f"{fn}{'d' if fn == 'validate' else 'ed'}_ckpt_path" | ||
| assert getattr(trainer, path_attr) is None |
There was a problem hiding this comment.
this feels fragile. can we be more explicit here?
There was a problem hiding this comment.
The test length would grow considerably (might as well split it into 3 tests then) if we don't use getattr. I know dynamic is ugly but I'd rather have a concise test
What does this PR do?
Fixes #7226 (comment)
Also fixes a bug where
predict_loop.on_trainer_init()was never called.Question
Should this go into 1.3.x? It could be considered a bug (it's inconsistent with the rest of the trainer fns)
Before submitting
PR review