Skip to content
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

support remove_input_padding for BertForSequenceClassification models #1834

Closed
wants to merge 3 commits into from
Closed

support remove_input_padding for BertForSequenceClassification models #1834

wants to merge 3 commits into from

Conversation

Altair-Alpha
Copy link
Contributor

Related issue: #1755
Content:

  1. Support remove_input_padding for BertForSequenceClassification models (implementation details given in code comment)
  2. Refinement of the build.py script, e.g., the original script doen't have a input model dir parameter, and will init the model with random weights, which is not intuitive.
  3. Since the model input is changed from 3 to 5, i.e. input_ids, input_lengths, token_type_ids, position_ids, max_input_length, I add a standalone run_remove_input_padding.py demo script, and show how to build them with only input_ids and token_type_ids.

I only implemented and tested this for BertForSequenceClassification but not other BERT models yet, please feel free to do further work on this :)

@Altair-Alpha
Copy link
Contributor Author

For our data, there's about 35% latency and 28% throughput enhancement when batch_size is 16, and the diff of output logits between the pytorch model and trt with remove_input_padding is below 0.01

@nv-guomingz
Copy link
Collaborator

Thanks @Altair-Alpha, we'll merge your changes and upstream to github next week.

examples/bert/build.py Outdated Show resolved Hide resolved
@nv-guomingz
Copy link
Collaborator

hi @Altair-Alpha , we've merged your PR into our internal code base with several minimal changes for internal ci.
This MR will be avaiable on next Tensor Tuesday.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants