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

Allow keeping default save_dir in ModelCheckpointer #1535

Closed
ematvey opened this issue Apr 20, 2020 · 2 comments · Fixed by #1548 or #1654
Closed

Allow keeping default save_dir in ModelCheckpointer #1535

ematvey opened this issue Apr 20, 2020 · 2 comments · Fixed by #1548 or #1654
Labels
feature Is an improvement or enhancement help wanted Open to be worked on

Comments

@ematvey
Copy link

ematvey commented Apr 20, 2020

Feature

Make filepath argument of ModelCheckpointer optional.

Motivation

I'm pretty happy with all defaults of ModelCheckpointer except save_top_k. In case I want to override that parameter I have to write some awkward code related to figuring out the checkpointing path, which is normally only known at runtime:

DEFROOT = Path('/data/models/lightning')
logger = WandbLogger(name=net.hparams.run_name, project=net.hparams.project, save_dir=str(DEFROOT))
logger.watch(net)
_ = logger.experiment  # create an experiment to determine version
cp_template = str(DEFROOT / net.hparams.project / ('version_'+logger.version) / 'checkpoints' / '{epoch:04d}-{val_loss:.2f}-{other_metric:.2f}.pt')

checkpointer = ModelCheckpoint(
    filepath=cp_template,
    save_top_k=10,
    verbose=True,
    monitor='val_loss',
    mode='min',
)

trainer = Trainer(
    gpus=1,
    logger=logger,
    default_root_dir=DEFROOT,
    checkpoint_callback=checkpointer,
    **extra,
)
trainer.fit(net)

It would be nice to have an option to allow Lightning to determine runtime-valued save location instead. Additionally, it would be nice to have an option to override checkpoint filename without overriding the whole save path.

@ematvey ematvey added feature Is an improvement or enhancement help wanted Open to be worked on labels Apr 20, 2020
@github-actions
Copy link
Contributor

Hi! thanks for your contribution!, great first issue!

@yukw777
Copy link
Contributor

yukw777 commented Apr 28, 2020

The fix to this issue introduced a bug.. if you don’t pass in filepath and use the default None, this check will fail b/c you can’t run os.path.isdir() on None

https://github.com/PyTorchLightning/pytorch-lightning/blob/master/pytorch_lightning/callbacks/model_checkpoint.py#L89

cc @PyTorchLightning/core-contributors

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Is an improvement or enhancement help wanted Open to be worked on
Projects
None yet
2 participants