-
Notifications
You must be signed in to change notification settings - Fork 861
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
[multimodal] fix num_gpus #3070
Conversation
Job PR-3070-68fa0f3 is done. |
@@ -482,7 +482,7 @@ def predict( | |||
if predictor._problem_type == OBJECT_DETECTION: | |||
strategy = "ddp" | |||
|
|||
if strategy == "ddp" and predictor._fit_called: | |||
if strategy == "ddp" and predictor._fit_called and predictor._problem_type == OBJECT_DETECTION: |
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.
Have you tested if it works for other problem type? While working on object detection task, the blocker is a cuda barrier which I think may not only affect the detection problem type
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.
multi-gpu works for other problem types, since we are using DP. I think @zhiqiangdon would have more detail.
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.
multi-gpu works for other problem types, since we are using DP. I think @zhiqiangdon would have more detail.
Here we try to allow using DDP for other problem types. Right?
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.
this single-line change isn't quite related to DDP yet, actually it would be DP for other problem types.
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.
This if condition is about ddp
. Wondering how it affects dp
.
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.
Yes, you're right, this if
condition is about ddp, but it should only affect OBJECT_DETECTION problem type, not other problem types.
it shouldn't affect dp for other problem types.
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
Issue #, if available:
num_gpus always become 1 when calling predictor.evaluate after fit, therefore prevent us from supporting multi-gpu.
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.