EPath: Implement forking directly and fix serialization #62
Conversation
…l functions. Also fix serialization of epath
| s3_args: Optional[Dict[str, Any]] | ||
|
|
||
| def __init__( | ||
| self, | ||
| initial_path: Union[str, "EPath", PathlibPath], | ||
| config_override: Optional[Dict[str, ConfigEntry]] = None, | ||
| *, | ||
| _test_config_override: Optional[Dict[str, ConfigEntry]] = None, |
There was a problem hiding this comment.
Suggest to rename to config_override since not only for tests, but config can be provided by user directly.
There was a problem hiding this comment.
I would make that unsupported. Because that'd mean we serialize the config along every EPath. I think every override should be made in the global config. Also, we'll anyways transition to msc.
There was a problem hiding this comment.
But even for msc, I think it makes sense to have the option to provide a config live if you don't want to have the common file on disk. The config should then only be serialized if it was overridden.
| else: | ||
| ignore_obj = None | ||
| EPath._after_fork_global() | ||
| for object in gc.get_objects(): |
There was a problem hiding this comment.
Wow, I'm not sure if I like this loop. We didn't need the GC before. I prefer the fork guard that we had before which is called on-demand.
There was a problem hiding this comment.
It's something you forget quickly. If any user e.g. parallelizes some method we offer, it'll break. This is automatic, nothing needed to think about.
There was a problem hiding this comment.
(and we also do that for the torch tensors)
There was a problem hiding this comment.
The loop can be reverted without the user having to remember anything, right? This is about fork_guard() not about prepare_forked_process()
EPath.prepare_forked_processafter forkingCloses #48