Changes for enabling checkpoint syncing for hyperopt #2115
Conversation
ShreyaR
force-pushed
the
hyperopt-checkpointing
branch
7 times, most recently
from
300bb16
to
1315130
Compare
|
|
||
| return (node_name, trial_dir) | ||
|
|
||
| @lru_cache(maxsize=1) |
There was a problem hiding this comment.
Can the node address change? I imagine if the ray train job can be made to be fault tolerance to head failure, it could.
There was a problem hiding this comment.
This is an interesting question. I expect that fault tolerance to head node failure for tune jobs is the only scenario in which head node IP will change.
However, in that case I would expect the HyperoptExecutor class to be initialized again, and so would have the correct head node address?
Re: caching here -- the returned value of this function doesn't depend on the head node IP at all, and so even if the head node were to change and we use the cached return val of this function, we'd still have the correct behavior.
Let me know if this makes sense or if there's something I can do to make this more robust.
There was a problem hiding this comment.
I think that sounds reasonable.
ShreyaR
force-pushed
the
hyperopt-checkpointing
branch
from
d587f92
to
f60e0ab
Compare
ShreyaR
force-pushed
the
hyperopt-checkpointing
branch
2 times, most recently
from
178fc8d
to
69dd174
Compare
ShreyaR
force-pushed
the
hyperopt-checkpointing
branch
from
4d93657
to
7d448b4
Compare
Changes necessary in order to use a KubernetesSyncer in order to sync checkpoints across pods.
The proposed changes use the storage on the node running the main process as the default storage location where all checkpoints are synced to.
I've left some comments in there right now mostly for debugging, which I plan to remove before merging this PR.