Skip to content

Add instance norm #573

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

Merged
merged 5 commits into from
Aug 20, 2021
Merged

Add instance norm #573

merged 5 commits into from
Aug 20, 2021

Conversation

zsef123
Copy link
Contributor

@zsef123 zsef123 commented Aug 11, 2021

Description

Add InstanceNorm usingTensorRT InstanceNormalizationPlugin and implementation of BatchNorm

Fixes #210

Also I checked #285
But this PR not supported TensorRT 8++
And it's a little different from mine.

  1. default beta value, BN and feat(//core/): Add support for tensorrt plugin and implement instance… #285 set to 1 but I think that must be 0
  2. type of eps, float is right type. And also my env, double occurs error.
  3. I called TensorRT directly not modifying other context (follows gelu)

Type of change

Please delete options that are not relevant and/or add your own.

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

Checklist:

  • My code follows the style guidelines of this project (You can use the linters)
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas and hacks
  • I have made corresponding changes to the documentation
  • I have added tests to verify my fix or my feature
  • New and existing unit tests pass locally with my changes

@zsef123 zsef123 force-pushed the add_instance_norm branch from dcc289e to fd787e0 Compare August 12, 2021 03:07
@zsef123
Copy link
Contributor Author

zsef123 commented Aug 12, 2021

@narendasan passed DCO

@narendasan
Copy link
Collaborator

@zsef123 Thanks for the contribution! Can you rebase your branch? Changes look mostly fine, once you rebase, I will run the tests and we should be good to go

@zsef123 zsef123 force-pushed the add_instance_norm branch from fd787e0 to 16c78e9 Compare August 13, 2021 01:52
@zsef123
Copy link
Contributor Author

zsef123 commented Aug 13, 2021

@zsef123 Thanks for the contribution! Can you rebase your branch? Changes look mostly fine, once you rebase, I will run the tests and we should be good to go

Checked!!

Signed-off-by: AhnDW <[email protected]>
@zsef123 zsef123 force-pushed the add_instance_norm branch from 4edc74a to 31d3b90 Compare August 18, 2021 17:06
@narendasan
Copy link
Collaborator

I am getting test failures for Converters.ATenInstanceNormRunningStatsConvertsCorrectly where the TRT implementation returns NaN. Using TRT8

Signed-off-by: AhnDW <[email protected]>
@zsef123
Copy link
Contributor Author

zsef123 commented Aug 19, 2021

@narendasan Sorry for missed.

Several problems were mixed. All of them were test cases.

  1. range of running_mean and running_var
    In testcase, cause overflow when calculate gamma and bias.
    So changed randn -> ones, zeros. It is default value of instance_norm

  2. not clone parameters in test case
    fixed

And changes

  1. changed device
    It is not bug, but I just follows batch_norm
    Changed cpu to cuda when the track_running_stats==True

  2. readable graph in test

P.S.
I had a hard time creating a new docker environment.
Any plans for a distribution with NGC 21.07?

@narendasan
Copy link
Collaborator

@zsef123 check #578 for a 21.07 container. It uses a version of Pytorch slightly newer than 1.9 but other than that should work

Copy link
Collaborator

@narendasan narendasan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, tests pass, LGTM

Signed-off-by: Naren Dasan <[email protected]>
Signed-off-by: Naren Dasan <[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.

✨[Feature] Please support aten::instance_norm
2 participants