-
Notifications
You must be signed in to change notification settings - Fork 68
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
Raise inference failure exceptions in default handlers #883
Conversation
@@ -368,7 +368,7 @@ def inference(self, inputs: Input): | |||
outputs.add_property("content-type", "application/json") | |||
except Exception as e: | |||
logging.exception("DeepSpeed inference failed") | |||
outputs = Output().error((str(e))) | |||
raise e |
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.
If we rethrow the exception, we don't need to catch it at the first place
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.
yeah, I thought about it too. I did not remove it as I was not sure if the log statement right above provided any value.
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.
the exception will be caught at higher level and will be logged there
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.
done, updated
…ry#883) * raise inference failure exceptions in default handlers
…ry#883) * raise inference failure exceptions in default handlers
Description
Currently inference failure exceptions are suppressed in default handlers preventing OOM exceptions to be handled correctly.
Output
is constructed here anyway.This is needed for MME on SM