-
Notifications
You must be signed in to change notification settings - Fork 8
Henryh/yemen crop #270
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
base: master
Are you sure you want to change the base?
Henryh/yemen crop #270
Conversation
| def init_mp() -> None: | ||
| """Set start method to preload and configure forkserver preload.""" | ||
| multiprocessing.set_start_method("forkserver", force=True) | ||
| multiprocessing.set_forkserver_preload( |
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.
Can we keep the preload, in case ? It makes it much faster if forkserver is used.
| def init_mp(context: str | None = None, sharing_strategy: str | None = None) -> None: | ||
| """Set start method for multiprocessing. | ||
|
|
||
| Uses RSLEARN_MULTIPROCESSING_CONTEXT if provided, else defaults to forkserver. |
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 think this could be clarified a bit. Also context does not seem like the correct name, should it be mp_start_method instead? Something like:
Args:
mp_start_method: the multiprocessing start method to set. If not set, use the RSLEARN_MULTIPROCESSING_CONTEXT if provided. Otherwise, it defaults to forkserver.
sharing_strategy: the torch sharing strategy to set. If not set, use the MP_SHARING_STRATEGY_ENV_VAR if provided. Otherwise, the sharing strategy is not changed.|
|
||
|
|
||
| if __name__ == "__main__": | ||
| multiprocessing.set_start_method("forkserver") |
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.
init_mp should be called here since we only call it in the entrypoints, not in the main functions. Moving init_mp into the main function in rslearn_main.py could be okay too.
No description provided.