-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
[AIR] Add set_preprocessor
method to Checkpoint
#31721
[AIR] Add set_preprocessor
method to Checkpoint
#31721
Conversation
Signed-off-by: amogkam <[email protected]>
set_checkpoint
to Checkpoint
set_checkpoint
method to Checkpoint
Signed-off-by: amogkam <[email protected]>
Signed-off-by: amogkam <[email protected]>
Sorry, do you mean |
set_checkpoint
method to Checkpoint
set_preprocessor
method to Checkpoint
@Yard1 yes sorry-- udpated the title |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
||
def testObjectRefCheckpointSetPreprocessor(self): | ||
ckpt = Checkpoint.from_dict({"x": 1}) | ||
ckpt = ray.get(ray.put(ckpt)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this ckpt
is an object ref checkpoint. When I checkedckpt._obj_ref
here, I got None
.
Is this what we want?
obj_ref = ray.put({"x": 1})
ckpt = Checkpoint.from_object_ref(obj_ref)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
from_object_ref
is deprecated in favor of ray.get
?
Seems like the _obj_ref
path is not used anymore at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, yeah, you're right. I'll open a PR to remove the dead code.
Failing test is also failing on master, going to merge. |
Signed-off-by: amogkam [email protected] Adds a set_preprocessor method to the Checkpoint class, analogous to the existing get_checkpoint method. This is necessary to easily specify preprocessors to Checkpoints when not using any of the existing framework-specific checkpoints. Signed-off-by: Andrea Pisoni <[email protected]>
Signed-off-by: amogkam [email protected]
Adds a
set_preprocessor
method to theCheckpoint
class, analogous to the existingget_checkpoint
method. This is necessary to easily specify preprocessors to Checkpoints when not using any of the existing framework-specific checkpoints.Why are these changes needed?
Related issue number
Checks
git commit -s
) in this PR.scripts/format.sh
to lint the changes in this PR.