Skip to content

trainer.validated/tested/predicted_ckpt_path: mark private or deprecate #10630

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

Closed
jjenniferdai opened this issue Nov 19, 2021 · 5 comments
Closed
Assignees
Labels
checkpointing Related to checkpointing deprecation Includes a deprecation let's do it! approved to implement refactor

Comments

@jjenniferdai
Copy link
Contributor

jjenniferdai commented Nov 19, 2021

Proposed refactor / Motivation

As detailed in this comment: #10573 (comment):

Pitch

To clarify/emphasize this, we can either:

  1. Set to private. Also just need one attribute (e.g. trainer._ckpt_path or trainer._last_restored_ckpt_path).

  2. Can we deprecate these attributes entirely? checkpoint_connector gives us info on the exact path here: https://github.com/PyTorchLightning/pytorch-lightning/blob/master/pytorch_lightning/trainer/connectors/checkpoint_connector.py#L75

Additional context


If you enjoy Lightning, check out our other projects! ⚡

  • Metrics: Machine learning metrics for distributed, scalable PyTorch applications.

  • Lite: enables pure PyTorch users to scale their existing code on any kind of device while retaining full control over their own loops and optimization logic.

  • Flash: The fastest way to get a Lightning baseline! A collection of tasks for fast prototyping, baselining, fine-tuning, and solving problems with deep learning.

  • Bolts: Pretrained SOTA Deep Learning models, callbacks, and more for research and production with PyTorch Lightning and PyTorch.

  • Lightning Transformers: Flexible interface for high-performance research using SOTA Transformers leveraging Pytorch Lightning, Transformers, and Hydra.

cc @justusschock @awaelchli @akihironitta @rohitgr7 @ananthsub @ninginthecloud @tchaton

@jjenniferdai jjenniferdai changed the title trainer.validated/tested/predicted_ckpt_path`: mark private or deprecate trainer.validated/tested/predicted_ckpt_path: mark private or deprecate Nov 19, 2021
@ananthsub ananthsub added checkpointing Related to checkpointing deprecation Includes a deprecation labels Nov 19, 2021
@awaelchli
Copy link
Contributor

At the most we would need only one of these attributes. It does not make sense to have one dedicated to each trainer method.

@jjenniferdai
Copy link
Contributor Author

At the most we would need only one of these attributes. It does not make sense to have one dedicated to each trainer method.

updated this in pitch option 1 !

@awaelchli awaelchli assigned awaelchli and jjenniferdai and unassigned awaelchli Jan 11, 2022
@awaelchli
Copy link
Contributor

awaelchli commented Jan 11, 2022

Since lately we changed the resume arguments to ckpt_path, I would say trainer.ckpt_path is most reasonable.

@carmocca
Copy link
Contributor

I agree with ^

@carmocca carmocca added the let's do it! approved to implement label Jan 11, 2022
@awaelchli
Copy link
Contributor

If I understand correctly, #11696 addressed this. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
checkpointing Related to checkpointing deprecation Includes a deprecation let's do it! approved to implement refactor
Projects
None yet
Development

No branches or pull requests

4 participants