DataLoader: add error detection for worker_init_fn #20150
Conversation
pytorchbot
added
the
module: dataloader
ssnl
force-pushed
the
psutil_hi_pri
branch
2 times, most recently
from
fb41e26 to
66a8005
Compare
|
@apaszke Do you plan to review this or should I find someone else? |
apaszke
left a comment
apaszke
left a comment
There was a problem hiding this comment.
This looks good, but here's an idea: instead of guarding every single place where an error might be raised inside the worker function, wrap it in another one and have a catch-all for any errors that might get thrown. The code used to transmit exceptions to the master process can get isolated and called from both places to avoid duplication.
|
@apaszke Thanks for the suggestion. I've given your proposal some thought. It is a nice idea, but for implementation we seem to have to try-catch in initialization or specialize the first iteration. I don't think it is worthwhile since we only need to cover this particular worker loop pattern. If in future we need to have another similar loop, we should definitely make this more general. :) |
|
@pytorchbot merge this please |
pytorchbot
added
the
merge-this-please
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@ezyang is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
6 participants
This is an attempt to isolate unrelated changes from #19228 for easier review.