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

Add author info replication script in gatys_ecker_betghe_2016 #296

Merged
merged 3 commits into from
Dec 13, 2022

Conversation

jbueltemeier
Copy link
Collaborator

The replication of gatys_ecker_betghe_2016 shows clear differences in the results. For this reason, I have contacted the author. He provided helpful tips for improving the results. This information on the hyperparameters is integrated with this PR. The most important changes are:

  1. the use of the activations after the ReLU function.
  2. the use of layer_weights depending on the number of channels.
  3. random initialisation of the image and more optimisation steps.

Due to the random initalisation, an exact replication is no longer possible, but the results show a clear improvement and a more similar stylisation to the original images.

@jbueltemeier jbueltemeier requested a review from pmeier October 19, 2022 14:04
Copy link
Member

@pmeier pmeier left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the effort Julian!

He provided helpful tips for improving the results.

Just to be sure I understand correctly: with improvement you mean better alignment of our replicated results with what is reported in the paper, right? Because that is what we are striving for. General quality improvement of the images is not a goal here.

In general, change looks good. In the future we should probably look at removing impl_params: bool in favor of config: GatysConfig like

class GatysConfig(enum.Enum):
    PAPER = enum.auto()
    REFERENCE_IMPLEMENTATION = enum.auto()
    AUTHOR = enum.auto()

This would make it much easier to deal with such a situation. This would also cover the instance_norm: bool thing that we have for Ulyanov.

Comment on lines 153 to 155
# The ratio is between a larger and a smaller value, according to the
# author the larger value was 1e6 or 1e9 (1e9 dont work)
larger_value = 1e6
Copy link
Member

Choose a reason for hiding this comment

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

So here is still uncertainty and the author also couldn't give us the data he used for producing the results in the paper? Could you detail what doesn't work for 1e9?

Copy link
Member

Choose a reason for hiding this comment

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

As discussed offline, the author provided these two discrete values. We'll recheck 1e9, but otherwise 1e6 is the "right" one.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

  1. I have looked at the commentary again and the ratio here refers to the weight ratio given in the paper. For this, there must be a larger value and a smaller value, and according to the author, the larger value (weight for style_loss) was the discrete value 1e6 or 1e9.
  2. The optimisation is unstable with a weighting of 1e9. The following error message appears regularly:
File "/home/julian/github/pystiche_papers/.venv/lib/python3.7/site-packages/torch/autograd/__init__.py", line 175, in backward
    allow_unreachable=True, accumulate_grad=True)  # Calls into the C++ engine to run the backward pass
RuntimeError: Function 'ConvolutionBackward0' returned nan values in its 1th output.`

replication/gatys_ecker_bethge_2016/main.py Outdated Show resolved Hide resolved
@pmeier pmeier self-requested a review November 18, 2022 10:55
Copy link
Member

@pmeier pmeier left a comment

Choose a reason for hiding this comment

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

Thanks!

@pmeier pmeier merged commit a8a87f4 into pystiche:main Dec 13, 2022
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.

2 participants