-
Notifications
You must be signed in to change notification settings - Fork 7.1k
[NOMERGE] AugMix inplace mul #6861
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -318,10 +318,6 @@ def posterize(inpt: features.InputTypeJIT, bits: int) -> features.InputTypeJIT: | |
|
||
|
||
def solarize_image_tensor(image: torch.Tensor, threshold: float) -> torch.Tensor: | ||
bound = 1 if image.is_floating_point() else 255 | ||
if threshold > bound: | ||
raise TypeError(f"Threshold should be less or equal the maximum value of the dtype, but got {threshold}") | ||
|
||
Comment on lines
-321
to
-324
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @pmeier This check didn't allow me to run the benchmark for float types so I removed it:
This is caused by the 255 values hardcoded on the Solarize linspace of AutoAugment. Now that we actually support float32 in all kernels, how do you propose to solve this? I think this should be disconnected from the PR at #6830. We could implement an approach to switch to 1.0 and 255 depending on the type of the input. Later if the aforementioned PR gets merged, we can update with more fine-grained max values that support all integers. Thoughts? |
||
return torch.where(image >= threshold, invert_image_tensor(image), image) | ||
|
||
|
||
|
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Turns out this is not beneficial and just complicates the implementation. See benchmark:
I think we should just remove the TODO. @vfdev-5 / @pmeier could you clip this on your next PR?