-
Notifications
You must be signed in to change notification settings - Fork 205
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
Optimizations and WAs to support HPU execution for Detr-Resnet-50 #1334
base: main
Are you sure you want to change the base?
Conversation
This PR builds on #1155 , which is meant for Eager mode and adds changes necessary for Lazy mode execution. Some of the review feedback for the former PR is addressed here. More details below. [x] pls. rebase/sync on top of main OH. |
a31ae42
to
0237edd
Compare
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.
Please rebase to latest main, there are some changes in modeling_utils.py
""" | ||
Copied from https://github.com/huggingface/transformers/tree/v4.40.2 | ||
https://github.com/huggingface/transformers/blob/4fdf58afb72b0754da30037fc800b6044e7d9c99/src/transformers/models/detr/modeling_detr.py#L2287 | ||
The modications are: |
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 modications are: | |
The modifications are: |
|
||
# Compute the classification cost. Contrary to the loss, we don't use the NLL, | ||
# but approximate it in 1 - proba[target class]. | ||
# The 1 is a constant that doesn't change the matching, it can be ommitted. |
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 1 is a constant that doesn't change the matching, it can be ommitted. | |
# The 1 is a constant that doesn't change the matching, it can be omitted. |
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.
@splotnikv , please take a look if you're covering for Sandeep
Done. I don't have access right to update this PR, so created a new one. See #1404. |
0237edd
to
4838a93
Compare
Rebased to latest optimum-habana, addressed feedback from #1404 , and merged in changes from that PR. Now that I'm back working on this, will use this PR going forward to complete the review process. Sorry for the back-and-forth over the two PR's. |
@sandeep-maddipatla Sorry was not able to work last few days so unable to review sooner.
Lazy mode passes
Pls clarify how the testing is to be done. |
@sandeep-maddipatla , could you update by EOW? |
@sandeep-maddipatla please resolve merge conflicts |
@sandeep-maddipatla , Please make changes and post results of retesting, otherwise this will be pushed to the 1.20 release. |
@sandeep-maddipatla If we don't see an update by wed, this will need to be part of 1.20 release. |
- Add capability to ignore targets that have an out-of-range ID - This helps to pad target objects to avoid graph recompilation and yet not affect the loss computation in training.
4838a93
to
d4d9fda
Compare
Sorry for the delayed update here. It appears that the
|
Test results with using the
|
Modifications to the Detr transformer including WA's and Optimizations to run the Detr-Resnet-50 model in eager and lazy modes on the HPU.
Fixes # (issue)
Before submitting