-
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
[tune] Test example checkpointing #4728
[tune] Test example checkpointing #4728
Conversation
Test PASSed. |
Test PASSed. |
Test PASSed. |
Test FAILed. |
Test FAILed. |
Test FAILed. |
Test PASSed. |
Test PASSed. |
Test FAILed. |
@hartikainen this is also related to your PR |
Test FAILed. |
python/ray/tune/trainable.py
Outdated
tmpdir = info["savedir"] | ||
if not os.path.exists(tmpdir): | ||
# We recreate the same subtree and path. | ||
os.makedirs(tmpdir) |
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.
Why change this logic? This will try to recreate the absolute path right? Whereas, the previous code recreated it relative to the given tempdir.
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.
So this was one way to enable tf.Saver
functionality.
saver = tf.Saver()
prefix = self.saver.save(self.sess, prefix=checkpoint_dir + "/save", global_step=self.iterations)
tf.Saver
saves things into a given directory with this format:
╰─ ls /Users/rliaw/ray_results/mnist_hyperband_test/TrainMNIST_0_learning_rate=0.00022789_2019-05-01_15-21-20lzl3rbbe/tmpudqrk34zsave_to_object/checkpoint_2/
checkpoint save-2.data-00000-of-00001 save-2.index save-2.meta save-2.tune_metadata
And, in checkpoint
, it saves absolute paths:
╰─ cat /Users/rliaw/ray_results/mnist_hyperband_test/TrainMNIST_0_learning_rate=0.00022789_2019-05-01_15-21-20lzl3rbbe/tmpudqrk34zsave_to_object/checkpoint_2/checkpoint
model_checkpoint_path: "/Users/rliaw/ray_results/mnist_hyperband_test/TrainMNIST_0_learning_rate=0.00022789_2019-05-01_15-21-20lzl3rbbe/tmpudqrk34zsave_to_object/checkpoint_2/save-2"
all_model_checkpoint_paths: "/Users/rliaw/ray_results/mnist_hyperband_test/TrainMNIST_0_learning_rate=0.00022789_2019-05-01_15-21-20lzl3rbbe/tmpudqrk34zsave_to_object/checkpoint_2/save-2"
There's alternative workarounds for this problem (i.e., doing a os.chdir in trainable:save_to_object()
), but this seemed reasonable...
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.
Won't this break when I move it to some other machine which does have /Users/rliaw?
Test PASSed. |
Test FAILed. |
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.
Looks fine but I would still raise a ValueError on unexpected return
Test PASSed. |
Test FAILed. |
Test FAILed. |
Test PASSed. |
Test PASSed. |
Test FAILed. |
Test FAILed. |
Test FAILed. |
Test FAILed. |
Test FAILed. |
Test FAILed. |
Test FAILed. |
Test FAILed. |
Test FAILed. |
Test PASSed. |
What do these changes do?
Due to the rmtree of save_to_object, TF saving fails. This PR fixes that.
Related issue number
Linter
scripts/format.sh
to lint the changes in this PR.