-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
new version of neural type system #307
Conversation
Signed-off-by: Oleksii Kuchaiev <okuchaiev@nvidia.com>
Signed-off-by: Oleksii Kuchaiev <okuchaiev@nvidia.com>
This pull request introduces 801 alerts when merging 21e7f31 into 1fddca2 - view on LGTM.com new alerts:
|
Signed-off-by: Oleksii Kuchaiev <okuchaiev@nvidia.com>
Signed-off-by: Oleksii Kuchaiev <okuchaiev@nvidia.com>
This pull request introduces 501 alerts when merging b8f633f into 1fddca2 - view on LGTM.com new alerts:
|
Signed-off-by: Oleksii Kuchaiev <okuchaiev@nvidia.com>
This pull request introduces 501 alerts when merging ac5e7ec into b4aba85 - view on LGTM.com new alerts:
|
Signed-off-by: Oleksii Kuchaiev <okuchaiev@nvidia.com>
This pull request introduces 501 alerts when merging 0cec895 into b4aba85 - view on LGTM.com new alerts:
|
This pull request introduces 527 alerts when merging a296fc2 into ad32363 - view on LGTM.com new alerts:
|
Signed-off-by: Oleksii Kuchaiev <okuchaiev@nvidia.com>
This pull request introduces 3 alerts when merging ce5cb07 into ad32363 - view on LGTM.com new alerts:
|
Signed-off-by: Oleksii Kuchaiev <okuchaiev@nvidia.com>
This pull request introduces 3 alerts when merging e0742e8 into ad32363 - view on LGTM.com new alerts:
|
Signed-off-by: Oleksii Kuchaiev <okuchaiev@nvidia.com>
This pull request introduces 3 alerts when merging f549c7c into 47f7346 - view on LGTM.com new alerts:
|
This pull request introduces 3 alerts when merging a35c2b6 into 29efd5c - view on LGTM.com new alerts:
|
enable deployment test Signed-off-by: Oleksii Kuchaiev <okuchaiev@nvidia.com>
This pull request introduces 5 alerts when merging 8e3b2c2 into 18b528e - view on LGTM.com new alerts:
|
This pull request introduces 5 alerts when merging 7b0398b into 4f299f4 - view on LGTM.com new alerts:
|
Signed-off-by: Oleksii Kuchaiev <okuchaiev@nvidia.com>
Signed-off-by: Oleksii Kuchaiev <okuchaiev@nvidia.com>
This pull request introduces 6 alerts and fixes 1 when merging 5bf06af into 233a0fe - view on LGTM.com new alerts:
fixed alerts:
|
Signed-off-by: Oleksii Kuchaiev <okuchaiev@nvidia.com>
This pull request introduces 6 alerts and fixes 1 when merging 5ddb513 into 233a0fe - view on LGTM.com new alerts:
fixed alerts:
|
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.
Hi, this is a really good piece of work.
Major issue: removing comments explaining the port definitions will force the user to actually look into the code. IMO we should add those comments back and update them so will inform what a given module expects as input and/or produces as output.
Two minor issues:
- Dimension -> Channel
- order of arguments in NeuralType init
This pull request introduces 6 alerts and fixes 1 when merging 6f64078 into ce0cfe6 - view on LGTM.com new alerts:
fixed alerts:
|
Signed-off-by: Oleksii Kuchaiev <okuchaiev@nvidia.com>
This pull request introduces 6 alerts and fixes 1 when merging ecbec6c into d46374e - view on LGTM.com new alerts:
fixed alerts:
|
@tkornuta-nvidia and @yzhang123 I think I've addressed/responded to your comments. |
This pull request introduces 6 alerts and fixes 1 when merging e964e46 into d46374e - view on LGTM.com new alerts:
fixed alerts:
|
Signed-off-by: Oleksii Kuchaiev <okuchaiev@nvidia.com>
This pull request introduces 6 alerts and fixes 1 when merging b692692 into 33d14dc - view on LGTM.com new alerts:
fixed alerts:
|
Signed-off-by: Oleksii Kuchaiev <okuchaiev@nvidia.com>
This pull request introduces 7 alerts and fixes 1 when merging 196a248 into 33d14dc - view on LGTM.com new alerts:
fixed alerts:
|
Implements new neural types into NeMo.
Updates core, all collections and tests to work with new types