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

Fixed MultiplicativeNoise for case with image shape [h, w, 1] #793

Merged
merged 4 commits into from
May 15, 2021

Conversation

Dipet
Copy link
Collaborator

@Dipet Dipet commented Dec 16, 2020

@@ -2706,7 +2706,7 @@ def get_params_dependent_on_targets(self, params):
shape = [c]
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment is not related to the bugfix itself. It points to the possible bug in implementation, when per_channel = True. In this case, the condition check will set shape to shape = [c], which I read as "apply same noise vector of [c] channels to all pixels". Probably, it should be shape = [h,w,1] to apply image-level noise to all channels instead.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@BloodAxe
I a little bit confused what you mean.
Do you mean that in your mind condition per_channel means that we need to generate noise with shape [h, w]?
In my mind condition per_channel=True means that we will use c equal to image c shape when we produce noise tensor, else we use c=1.
If elementwise=True we set [h, w] of noise vector equal to image [h, w]. If elementwise=False noise vector h = w = 1

@Dipet Dipet requested review from BloodAxe and creafz May 15, 2021 16:57
@BloodAxe BloodAxe merged commit 1a35d2c into master May 15, 2021
@Dipet Dipet deleted the fix_multiplicative_noise branch May 20, 2021 09:25
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