[air] Add *path properties to Result and ResultGrid - #33410
Merged
Conversation
added 7 commits
March 16, 2023 19:43
Signed-off-by: Kai Fricke <kai@anyscale.com>
krfricke
added
the
tests-ok
Contributor
|
Hey, did we end up going through #api-changes for this? Also, this doesn't seem to correspond with our discussion on the PRD..? |
Signed-off-by: Kai Fricke <kai@anyscale.com>
krfricke
removed
the
tests-ok
added 3 commits
March 19, 2023 16:43
Signed-off-by: Kai Fricke <kai@anyscale.com>
krfricke
added
the
tests-ok
richardliaw
reviewed
Mar 21, 2023
Signed-off-by: Kai Fricke <kai@anyscale.com>
krfricke
removed
the
tests-ok
added 2 commits
March 21, 2023 17:50
Signed-off-by: Kai Fricke <kai@anyscale.com>
Signed-off-by: Kai Fricke <kai@anyscale.com>
ericl
reviewed
Mar 22, 2023
Signed-off-by: Kai Fricke <kai@anyscale.com>
justinvyu
reviewed
Mar 22, 2023
justinvyu
left a comment
justinvyu
left a comment
Contributor
There was a problem hiding this comment.
Thanks! Just added a few questions:
| setattr(self, attr, value) | ||
|
|
||
| @property | ||
| def path(self) -> Optional[str]: |
Contributor
There was a problem hiding this comment.
Should checkpoint.uri get deprecated given this one?
Also, what if a user wants the local path for some reason when using cloud? Can only access through private property right now.
Contributor
Author
There was a problem hiding this comment.
I think we should keep URI to have counterparts to to_uri and from_uri for now. But I agree, we should probably see if we want to move to to_path and from_path instead.
Contributor
Author
There was a problem hiding this comment.
For accessing the local path, if a checkpoint has a URI, the checkpoint object does not have a local path stored. There may be cached data somewhere on the local node, but it doesn't have to be (e.g. if the checkpoint was stored on a worker node). The user can use checkpoint.to_directory() to download the data to a target directory.
added 6 commits
March 22, 2023 18:40
Signed-off-by: Kai Fricke <kai@anyscale.com>
richardliaw
approved these changes
Mar 23, 2023
Signed-off-by: Kai Fricke <kai@anyscale.com>
elliottower
pushed a commit
to elliottower/ray
that referenced
this pull request
Apr 22, 2023
…t#33410) Following ray-project#33370, this PR adds `Result.path` and `ResultGrid.experiment_path` to the respective classes. Further, we remove the public facing `ExperimentAnalysis.local_path` and `ExperimentAnalysis.remote_path` in favor of a unified `ExperimentAnalysis.experiment_path`. Signed-off-by: Kai Fricke <kai@anyscale.com> Signed-off-by: elliottower <elliot@elliottower.com>
ProjectsByJackHe
pushed a commit
to ProjectsByJackHe/ray
that referenced
this pull request
May 4, 2023
…t#33410) Following ray-project#33370, this PR adds `Result.path` and `ResultGrid.experiment_path` to the respective classes. Further, we remove the public facing `ExperimentAnalysis.local_path` and `ExperimentAnalysis.remote_path` in favor of a unified `ExperimentAnalysis.experiment_path`. Signed-off-by: Kai Fricke <kai@anyscale.com> Signed-off-by: Jack He <jackhe2345@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why are these changes needed?
Following #33370, this PR adds
Result.pathandResultGrid.experiment_pathto the respective classes. Further, we remove the public facingExperimentAnalysis.local_pathandExperimentAnalysis.remote_pathin favor of a unifiedExperimentAnalysis.experiment_path.Related issue number
Checks
git commit -s) in this PR.scripts/format.shto lint the changes in this PR.