add os.Path support for load/save torch models#2947
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2947 +/- ##
==========================================
- Coverage 95.52% 95.48% -0.05%
==========================================
Files 146 150 +4
Lines 15703 16201 +498
==========================================
+ Hits 15000 15469 +469
- Misses 703 732 +29 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
dennisbader
left a comment
There was a problem hiding this comment.
Thanks a lot for this PR @turbotimon 🚀
Adding support for Path-like objects sounds great. In this case there are some more places where we should allow this, to provide a unified usage over all methods.
There was a problem hiding this comment.
Would be great to add this to all other save / load methods:
- model construction param:
work_dir to_onnx()load_weights()work_dirinload_from_checkpoint()work_dirinload_weights_from_checkpoint()
There was a problem hiding this comment.
yes that makes sense. Will try to do so as soon as i find time
Co-authored-by: Dennis Bader <dennis.bader@gmx.ch>
|
Would you like to continue the work on this? I think both supporting While Thanks to @dennisbader for leading me here. |
|
@daidahao thanks for asking and sorry for the late reply
Actually, yes, but I'm pretty busy right now… So I don't mind if someone else takes care of it; otherwise, I'll take another look at it as soon as I have some free time.
I do not think that is related to "file-like object", it's just a representative of a path, not a real file. And .ckpt gets added by convention. Bundling can, however, be advantageous for other reasons, but I don't have a strong opinion on that. |
Checklist before merging this PR:
Summary
Currently, the save/load methdos for TorchForecastingModel do not support os.PathLike object for the path argument. Other model-families (conformal, ensemble, forecasting) already do so. This PR changes that, improving the usability as well as make the interface more similar.
No additional test are needed. As the path argument is transformed to a pathlib.Path object at the beginning it is guaranteed to work with str as well as any os.PathLike objects
The other Model-family also support file-like objects (IObuffer). But since pl.Trainer.save_checkpoint does not supports it, i didn't add this support as well. Torch.save would support it, however.