Skip to content

Conversation

@amyeroberts
Copy link
Contributor

@amyeroberts amyeroberts commented Aug 23, 2022

What does this PR do?

The image segmentation pipeline tests - tests/pipelines/test_pipelines_image_segmentation.py - were failing after the merging of #1849 (49e44b2). This was due to the difference in rescaling.

Previously the images were rescaled by image = image / 255. In the new commit, a rescale method was added, and images rescaled using image = image * scale. This was known to cause small differences in the processed images (see
PR comment).

Testing locally, changing the rescale method to divide by a scale factor (255) resulted in the tests passing. It was therefore decided after discussion with @ydshieh the test values could be updated, as there was no logic difference between the commits.

Comparing masks

The same code as in the segmentation pipeline was run to compare the output masks.

#git_hash = "86d0b26d6c5566506b1be55002654b48d9c19ffe"
git_hash = "49e44b216b2559e34e945d5dcdbbe2238859e29b"

from transformers import pipeline
import numpy as np
model_id = "facebook/detr-resnet-50-panoptic"
image_segmenter = pipeline("image-segmentation", model=model_id)
outputs = image_segmenter("http://images.cocodataset.org/val2017/000000039769.jpg")
for i, o in enumerate(outputs):
    img = o['mask']
    np.save(f"img_{git_hash}_{i}", img)
    img.save(f"img_{git_hash}_{i}.png")

Mask generate for output 0 on commit hash 86d0b26 (parent commit)
img_86d0b26d6c5566506b1be55002654b48d9c19ffe_0

Mask generate for output 0 on commit hash 49e44b2
img_49e44b216b2559e34e945d5dcdbbe2238859e29b_0

Checking the pixel values, there was a single pixel difference

>>> git_hash_1 = "86d0b26d6c5566506b1be55002654b48d9c19ffe"
>>> git_hash_2 = "49e44b216b2559e34e945d5dcdbbe2238859e29b"
>>> 
>>> def max_diff(a, b):
>>>     return np.amax(np.abs(a - b))
>>> 
>>> for i in range(6):
>>>     print(f"\nComparing mask {i}")
>>>     img_orig = f"img_{git_hash_1}_{i}"
>>>     img_new = f"img_{git_hash_2}_{i}"
>>>     arr_orig = np.load(img_orig + ".npy")
>>>     arr_new = np.load(img_new + ".npy")
>>>     print(f"Max difference: ", max_diff(arr_orig, arr_new))
>>> 
>>>     m = arr_orig != arr_new
>>>     # image shape is 480 x 640
>>>     x = np.arange(480 * 640).reshape(480, 640)
>>>     print("No. pixels different: ", len(x[m]))

Output:

Comparing mask 0
Max difference:  255
No. pixels different:  1

Comparing mask 1
Max difference:  0
No. pixels different:  0

Comparing mask 2
Max difference:  0
No. pixels different:  0

Comparing mask 3
Max difference:  0
No. pixels different:  0

Comparing mask 4
Max difference:  0
No. pixels different:  0

Comparing mask 5
Max difference:  0
No. pixels different:  0

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline,
    Pull Request section?
  • Was this discussed/approved via a Github issue or the forum? Please add a link
    to it if that's the case.
  • Did you make sure to update the documentation with your changes? Here are the
    documentation guidelines, and
    here are tips on formatting docstrings.
  • Did you write any new necessary tests?

The image segmentation pipeline tests - tests/pipelines/test_pipelines_image_segmentation.py - were failing after the merging of huggingface#1849  (49e44b2). This was due to the difference in rescaling. Previously the images were rescaled by `image = image / 255`. In the new commit, a `rescale` method was added, and images rescaled using `image = image * scale`. This was known to cause small differences in the processed images (see
[PR comment](huggingface#18499 (comment))).

Testing locally, changing the `rescale` method to divide by a scale factor (255) resulted in the tests passing. It was therefore decided the test values could be updated, as there was no logic difference between the commits.
@amyeroberts amyeroberts requested a review from ydshieh August 23, 2022 10:26
@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Aug 23, 2022

The documentation is not available anymore as the PR was closed or merged.

Copy link
Collaborator

@ydshieh ydshieh left a comment

Choose a reason for hiding this comment

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

LVGTM, thank you @amyeroberts for checking all the details!

@ydshieh ydshieh requested a review from sgugger September 14, 2022 09:46
@ydshieh
Copy link
Collaborator

ydshieh commented Sep 15, 2022

@amyeroberts Let's merge this PR?

@sgugger sgugger merged commit 30a28f5 into huggingface:main Sep 15, 2022
@sgugger
Copy link
Collaborator

sgugger commented Sep 15, 2022

Amy is on vacation ;-)

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.

4 participants