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

Added a reflectance padding option from leongatys/NeuralImageSynthesis #377

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

ProGamerGov
Copy link

@ProGamerGov ProGamerGov commented Feb 11, 2017

The new command is used similar to the -normalize_gradients command, with: -reflectance. By default, it is set to false.

The related issue can be found here: #376

@htoyryla
Copy link

htoyryla commented Feb 12, 2017

See #376 (comment) and the continuation about a potential problem in this modification.

@ProGamerGov
Copy link
Author

This new feature should be working correctly now.

@ProGamerGov ProGamerGov reopened this Feb 12, 2017
@htoyryla
Copy link

This pull request is part of an experimental attempt to extend neural-style towards "Controlling Perceptual Factors in Neural Style Transfer" (see #376). I am not convinced of the value of changing the existing neural-style in this way; I would rather see parallel projects in which the basic neural-style code is developed to different directions with new functionality (like so many have already done).

My main reason for this is that neural-style is a very good starting point to explore implementing style transfer in lua. The code is as compact, clear and simple as it can be. Why not keep it that way. Adding experimental features into it will make it more difficult to understand and re-use, more error prone to develop further and even use. I have done numerous experiments based on this code, but preferred not to offer them into the main neural-style branch.

@VaKonS
Copy link

VaKonS commented Feb 14, 2017

@htoyryla, this particular "padding" option seems to be useful with small images – it removes dark spots near the border.
(And adds some noise, see bottom edge of second image. Didn't understand why yet).
Without and with padding:

no_padding reflection_padding

@htoyryla
Copy link

htoyryla commented Feb 14, 2017

Note that this is not my decision anyway, just an opinion. I can understand the addition of a set of padding options, if we see that padding really makes a difference. And perhaps it does... fast-neural-style does offer many padding options using customized padding layers, although it looks like they are only used in the feed-forward version, slow-neural-style.lua does not use them.

I would prefer a new parameter -padding, which could take several values:

  • the default being 'default' meaning "leave padding to the convolution layer".
  • 'reflect' meaning the new behaviour here (the word 'reflectance' sounds strange to me, nn uses 'reflection')

nn further offers replication padding, which could be added later (although there is little difference between reflection and replication when padding with one pixel.

@ProGamerGov
Copy link
Author

ProGamerGov commented Feb 14, 2017

@htoyryla Changing the -reflectance parameter to -padding should be easy enough, and if future padding options arise, a new command won't be needed.

Edit: The parameter is now known as -padding and currently takes either default or reflect, with default being the default option.

@VaKonS
Copy link

VaKonS commented Feb 15, 2017

@htoyryla, the error seems to accumulate, and difference in 1-pixel border makes noticeable changes after relatively high number of iterations (the above images are made with 300 iterations).

"Replication" variant makes difference with small details near the edges (reflection and replication):

reflection_padding replication_padding

"Reflection" eats those details, "replication" can stretch some elements to the border (see red ribbon on the left side). So both options make sense.

@htoyryla
Copy link

htoyryla commented Feb 15, 2017 via email

@htoyryla
Copy link

htoyryla commented Feb 15, 2017 via email

@VaKonS
Copy link

VaKonS commented Feb 15, 2017

@htoyryla, all three images are made with same settings, the only difference is padding option – default, reflection and replication.

The seed number is fixed too.

@htoyryla
Copy link

htoyryla commented Feb 15, 2017 via email

@VaKonS
Copy link

VaKonS commented Feb 15, 2017

@htoyryla, I see the point. I used "vgg_normalised" model and fixed seed number.
And tried several times with different styles and image sizes, so the results should not be accidental.

@htoyryla
Copy link

htoyryla commented Feb 15, 2017 via email

@htoyryla
Copy link

htoyryla commented Feb 15, 2017

The error statement is needed, not only to notify the user, but to stop execution because we don't know what exactly should be done. That's why an error() statement is used and not simply print().

@ProGamerGov
Copy link
Author

ProGamerGov commented Feb 15, 2017

@htoyryla Looking at the other uses of error() in Neural-Style, that makes sense. Thanks for the insight.

@VaKonS
Copy link

VaKonS commented Feb 17, 2017

I was wrong, the noise is not added by padding, it seems to be a feature of some models.
Here is a clean result of one model and noisy of another (default padding / reflection / replication):

s4-crowsonkb-100-300000-0 0-def-refl-repl
s2-illustration2vecall-10-1000-0 57-def-refl-repl

More tests in the discussion.

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.

3 participants