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

Sandeepkumar skb groupnorm plugin #437

Merged

Conversation

jaybdub
Copy link
Contributor

@jaybdub jaybdub commented Nov 2, 2020

No description provided.

@sandeepkumar-skb
Copy link
Contributor

Thanks @jaybdub; I reviewed the changes and also tested the changes with FP32 and FP16 mode. The max_error in both modes is 0.

Thanks,
Sandeep

@sandeepkumar-skb
Copy link
Contributor

Hi @jaybdub - Is there anything that needs to be done from my side?

@jaybdub
Copy link
Contributor Author

jaybdub commented Nov 4, 2020

Hi @sandeepkumar-skb,

I think it should be good! Going to do some sanity checks and then likely merge.

Thanks again for the PR! Please let me know of any PRs / issues in the future.

Best,
John

@sandeepkumar-skb
Copy link
Contributor

sandeepkumar-skb commented Nov 4, 2020

Absolutely! Planning to add more support in future.

Thanks,
Sandeep

@jaybdub jaybdub merged commit d1fa6f9 into NVIDIA-AI-IOT:master Nov 4, 2020
@jaybdub
Copy link
Contributor Author

jaybdub commented Nov 4, 2020

It's merged!

Do you mind sharing the use-case for GroupNorm layer? I personally haven't used this layer in my models.

I'm curious how the performance of the plugin is in context. Depending on the performance cost from copying the output, it may be worth implementing an in-place version natively at some point in the future.

Best,
John

@sandeepkumar-skb
Copy link
Contributor

We have models which use GroupNorm and converting them to TensorRT for inference is a major challenge because it is not supported in TensorRT yet and there is no automatic fallback to framework like in TF-TRT. We then have 2 options:

  1. Replace GroupNorm with other Normalizations and retrain - Not ideal!!
  2. Exclude the block with uses GroupNorm from TRTfying so we are leaving performance on the table.
    Adding this plugin will enable TRTifying the block which is TRT compatible and GroupNorm would just fallback to PyTorch.

I have yet to measure the performance but certainly this is not an ideal solution. Having a native TRT support for this op will be best.

jaybdub added a commit that referenced this pull request Jun 28, 2021
* added plugin for GroupNorm

Co-authored-by: sandeepkumar-skb <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants