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

[Relay] Keras frontend upsample and 1 channel conv2d fixes #3937

Merged
merged 3 commits into from
Sep 18, 2019

Conversation

jwfromm
Copy link
Contributor

@jwfromm jwfromm commented Sep 11, 2019

This Discussion post found two bugs in Relay. The first was a silly mistake I introduced with the Resize rework (#3788 ) caused by defaulting to the wrong layout for upsample in the Keras frontend. I've resolved this issue and fixed the corresponding test. The second bug occurs when a convolution with 1 filter attempts to determine the shape of its weights. Relay checks for group convolutions by seeing if the number of channels equals the group size, however we obviously shouldn't if the number of channels is 1. I've added a new test to check for this failure in the future.

@jwfromm jwfromm changed the title [Relay] Keras Frontend Upsample and Conv with channels=1 bug fixes [Relay] Keras frontend upsample and 1 channel conv2d fixes Sep 11, 2019
@tqchen
Copy link
Member

tqchen commented Sep 13, 2019

@kazum please manage the PR , @jwfromm please request reviews from folks by tagging them next time

@jwfromm
Copy link
Contributor Author

jwfromm commented Sep 16, 2019

@kazum, can you take a look? These are pretty minor fixes so it should be quick.

@jwfromm
Copy link
Contributor Author

jwfromm commented Sep 17, 2019

@masahi, maybe you can take a look instead?

@jwfromm jwfromm closed this Sep 17, 2019
@jwfromm jwfromm reopened this Sep 17, 2019
@kazum
Copy link
Contributor

kazum commented Sep 17, 2019

@jwfromm, sorry for being late. I'll take a look today.

Copy link
Contributor

@kazum kazum left a comment

Choose a reason for hiding this comment

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

Looks good, thanks @jwfromm !

@kazum kazum merged commit de12376 into apache:master Sep 18, 2019
wweic pushed a commit to wweic/tvm that referenced this pull request Sep 30, 2019
* Fix upsample layout in keras frontend.

* Fixed group conv being used instead of conv when channels=1

* Add new conv2d test to catch bugs when channels=1.
wweic pushed a commit to wweic/tvm that referenced this pull request Sep 30, 2019
* Fix upsample layout in keras frontend.

* Fixed group conv being used instead of conv when channels=1

* Add new conv2d test to catch bugs when channels=1.
wweic pushed a commit to neo-ai/tvm that referenced this pull request Oct 1, 2019
* Fix upsample layout in keras frontend.

* Fixed group conv being used instead of conv when channels=1

* Add new conv2d test to catch bugs when channels=1.
@jwfromm jwfromm deleted the keras_upsample_fix branch January 11, 2020 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants