-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Allow start_method
s other than fork on MultiprocessParallelUpdater
#7552
Conversation
This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 30 days if no further activity occurs. Thank you for your contributions. |
This issue is closed as announced. Feel free to re-open it if needed. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 30 days if no further activity occurs. Thank you for your contributions. |
'MultiprocessParallelUpdater assumes the context is ' | ||
'uninitialized. Please do not call CUDA API before ' | ||
'MultiprocessParallelUpdater creates processes.') | ||
if multiprocessing.get_start_method() != 'spawn': |
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.
- Check
'forkserver'
. (Thanks @kmaehashi) if _cuda_initialized and multiprocessing.get_start_method() != 'spawn':
?
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.
Update on this.
forkserver
also works.
There is a problem when using spawn
and forkserver
the converter
attribute of the MultiprocessParallelUpdater
can't be pickled.
Either we define all the converters as classes -> no compat and horrible approach.
Or we find some way around
f75e17e
to
ee9fe53
Compare
tests/chainer_tests/training_tests/updaters_tests/snippets/cuda_init.py
Outdated
Show resolved
Hide resolved
…a_init.py Co-Authored-By: Toshiki Kataoka <tos.lunar@gmail.com>
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
The PR itself looks good, but could you change its title to clarify the change makes the usage more permissive, e.g. "Allow |
Jenkins, test this please. |
MultiprocessParallelUpdater
start_method
s other than fork on MultiprocessParallelUpdater
Thanks!🙇 |
Jenkins CI test (for commit 2068699, target branch master) succeeded! |
If a cuda context is created before running
MultiprocessParallelUpdater
it should fail.multiprocessing.set_start_mode('spawn')
avoids the cuda context duplication on the worker processes even if we have performed cuda operations on the main thread such as getting the number of available GPUs.#7309 is related.