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

Fix PixelShuffle_icnr weight #3322

Merged
merged 4 commits into from
Apr 30, 2021
Merged

Fix PixelShuffle_icnr weight #3322

merged 4 commits into from
Apr 30, 2021

Conversation

pratX
Copy link
Contributor

@pratX pratX commented Apr 14, 2021

When using weight_norm (the current default), PixelShuffle_icnr needs to icnr_init weight_g and weight_v.
Issue #3315

…ight_norm and icnr_init modifies derived weight instead of weight_g and weight_v
@pratX
Copy link
Contributor Author

pratX commented Apr 15, 2021

Work in Progress. Checking out nbdev and following the development process used for fastai.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@pratX
Copy link
Contributor Author

pratX commented Apr 15, 2021

Updated the notebook with the fix and also the unit test. The unit test passes with weight_norm enabled (earlier weight_norm was explicitly disabled in the unit test, even though the description said that weight_norm is what worked best with super_resolution) which is the expected behavior.

@pratX pratX marked this pull request as ready for review April 15, 2021 06:12
@pratX
Copy link
Contributor Author

pratX commented Apr 15, 2021

Updated PixelShuffle_icnr unit test to check for both cases, weight_norm enabled and disabled. Both the unit tests pass.

@tcapelle
Copy link
Contributor

tcapelle commented Apr 16, 2021 via email

@pratX
Copy link
Contributor Author

pratX commented Apr 17, 2021

Would you mind adding the spectral norm init also?

On Thu, Apr 15, 2021, 8:50 AM pratX @.***> wrote: Updated PixelShuffle_icnr unit test to check for both cases, weight_norm enabled and disabled. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#3322 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEMWOAK4ANPJKXU26YDGKX3TI2EDDANCNFSM42542OBA .

Added unit test for PixelShuffle_icnr with sepctral_norm.

@tcapelle
Copy link
Contributor

graet!

@jph00
Copy link
Member

jph00 commented Apr 30, 2021

Many thanks!

@jph00 jph00 merged commit a3c2c5e into fastai:master Apr 30, 2021
@hamelsmu hamelsmu changed the title Fix Issue #3315: PixelShuffle_icnr icnr_init with weight_norm Fix PixelShuffle_icnr weight Apr 30, 2021
@hamelsmu hamelsmu added the bug label Apr 30, 2021
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.

None yet

4 participants