Local dir cli #146
Local dir cli #146
Conversation
There was a problem hiding this comment.
Hey @brandontrabucco, thanks a lot for opening this! Can you just do a test that the local dir works as expected? E.g. with ~/ray_results, ~/ray_results/some/folder, and some folder that is outside of ~/ray_results.
| @@ -80,8 +80,7 @@ def add_command_line_args_to_variant_spec(variant_spec, command_line_args): | |||
|
|
|||
|
|
|||
| def generate_experiment_kwargs(variant_spec, command_line_args): | |||
| # TODO(hartikainen): Allow local dir to be modified through cli args | |||
| local_dir = '~/ray_results' | |||
| local_dir = command_line_args.local_dir | |||
There was a problem hiding this comment.
This looks good to me. Just for my own future reference: If I recall correctly, the reason I originally didn't do this was that there's some edge case where not using the default ~/ray_results local directory made the results get synced to weird places. I can't figure out now what the problem was, so maybe it's been fixed e.g. on the Ray side.
|
Thanks for the comment @hartikainen I have tested using a local-dir in a './data' folder rather than In any case, the default value for the '--local-dir' command line argument is '~/ray_results' so users that depend on consistent behavior between the old local_dir and upload_dir variables should not be impacted, by default. |
Co-authored-by: Kristian Hartikainen <kristian.hartikainen@gmail.com>
|
Let me know if you'd like me to test anything else before we merge this feature in. |
|
Hey @brandontrabucco. Yeah, I think it's important to test that this doesn't mess up the cluster paths too badly. If possible, could you try testing at least the following combinations?
Once those look reasonable, I'm happy to merge this 🙂 |
|
@brandontrabucco I just tested the combinations above and they seem to work as expected. Thanks a lot for contributing this! |
|
Thanks for doing that! |
This pull request adds a --local-dir argument to the command line interface and resolves this issue: #128. I also fixed an issue with pip install -e . that re-installs an older incompatible version of ray.