-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
LightningCLI: Support config files on object stores #7360
Comments
@leezu I think this is a good idea, will look into it. |
Had a quick look at this and I have the following thoughts/doubts:
|
Do you think this should be done here or in upstream jsonargparse?
I guess environment variables are the best option here
Not sure if it's worth for us to have those tests instead of just trusting upstream fsspec |
Manually setting environment variables is possible, but may not be the so convenient :) If your code is running within an AWS network (eg. EC2), the credentials can be automatically retrieved (assuming your instance is configured to have access to the bucket in question) and fsspec will "just work". See https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/iam-roles-for-amazon-ec2.html#instance-metadata-security-credentials |
jsonargparse already supports http/https for the config, though requires optional packages and be explicitly enabled. The fsspec support should be added extending that. Here what we can do is enable fsspec by default, since it can be assumed that fsspec is installed.
I am interested in looking at code and tests using fsspec. What I was able to find in lightning is not enough to know how to best do this. I have not used fsspec. It looks very powerful but the documentation seems to lack a bit.
That is nice. Though I think the scope of this shouldn't be to make it work only for s3 on aws. If someone wants to use this in some private cluster which uses a minio server as storage, would fsspec "just work"? Or what if someone wants to use some other file system supported by fsspec? How would users know how to do this or that this is possible? The documentation in lightning and in fsspec is not enough. Or maybe I missed where these things are explained better. @leezu when you say "environment variables is possible" do you mean that we could implement it like this, or fsspec already supports environment variables? Even if we initially implement it so that it works only for s3 on aws it would be good to have an idea how it can be extended without needing completely change the original implementation. |
fsspec just calls the AWS SDK for Python which then attempts to find credentials. Above example just serves as documentation to understand how the SDK does that. Sorry if it was confusing. The takeaway is that you don't need to worry about authentication in jsonargparse/pytorch-lightning and that each of the fsspec backends should have their own ways (via environment variables or other features) to ensure authentication is taken care of. |
I have implemented the fsspec support in jsonargparse. @leezu would you mind trying it out with from jsonargparse import set_config_read_mode
set_config_read_mode(fsspec_enabled=True) |
Thank you @mauvilsa. It works great! |
Closing this issue, feel free to reopen if there's anything to be done on the lightning front. |
@edenlightning it'd be helpful for lightning to call
as lightning already depends on fsspec |
@leezu Want to open a PR with it? |
There isn't yet a jsonargparse release with the fsspec support. When this gets released I can create the PR. |
🚀 Feature
In addition to
python trainer.py --config config.yaml
, LightningCLI should enablepython trainer.py --config s3://bucket/config.yaml
Motivation
pytorch-lightning has good support for remote filesystems thanks to #2164 #3320 (and possibly others). Thanks to that, it's straightforward to deploy lightning-based scripts to compute cluster environments without having to write extra code preparing / post-processing the local file-systems https://pytorch-lightning.readthedocs.io/en/latest/clouds/cluster.html It would be nice to extend this to LightningCLI.
When using LightningCLI however, it's currently necessary to deploy the config.yaml file to each node, instead of referencing a copy in an object store such as S3.
cc @mauvilsa?
The text was updated successfully, but these errors were encountered: