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

Fix BeitFeatureExtractor postprocessing #19119

Merged
merged 11 commits into from
Sep 20, 2022

Conversation

alaradirik
Copy link
Contributor

What does this PR do?

  • Fixes a BeitFeatureExtractor.post_process_semantic_segmentation() assertion error when no target_sizes argument is provided
  • Ensures post_process_semantic_segmentation returns a list of int64 PyTorch tensors
  • Adds a test to ensure correct post-processing

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • [X ] 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.
  • [X ] Did you make sure to update the documentation with your changes? Here are the
    documentation guidelines, and
    here are tips on formatting docstrings.
  • [X ] Did you write any new necessary tests?

@alaradirik alaradirik changed the title Beit postprocessing Fix BeitFeatureExtractor postprocessing Sep 20, 2022
resized = self.resize(image=semantic_segmentation[idx], size=target_sizes[idx])
resized_maps.append(resized)

semantic_segmentation = [torch.Tensor(np.array(image)).to(torch.int64) for image in resized_maps]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
semantic_segmentation = [torch.Tensor(np.array(image)).to(torch.int64) for image in resized_maps]
semantic_segmentation = [torch.Tensor(np.array(image)).long() for image in resized_maps]

This should also work :)

None, predictions will not be resized.
Returns:
semantic_segmentation: `List[torch.Tensor]` of length `batch_size`, where each item is a semantic
segmentation map of shape (w, h) corresponding to the target_sizes entry (if `target_sizes` is specified).
Copy link
Contributor

Choose a reason for hiding this comment

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

To me it's a bit counterintuitive to output (w, h) if we ask the target_sizes to be (h, w)

=> so I'd use the same format for both

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, just fixed this

semantic_segmentation = semantic_segmentation.numpy()

for idx in range(len(semantic_segmentation)):
resized = self.resize(image=semantic_segmentation[idx], size=target_sizes[idx])
Copy link
Contributor

@NielsRogge NielsRogge Sep 20, 2022

Choose a reason for hiding this comment

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

As discussed offline, please use nn.functional.interpolate here.

resized = nn.functional.interpolate(semantic_segmentation[idx],
                size=target_sizes[idx],
                mode='bilinear',
                align_corners=False)

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Sep 20, 2022

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

Copy link
Contributor

@NielsRogge NielsRogge left a comment

Choose a reason for hiding this comment

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

LGTM, although it will require an update when supporting the TF model.

@alaradirik alaradirik merged commit 36b9a99 into huggingface:main Sep 20, 2022
@LysandreJik
Copy link
Member

Hey @alaradirik, please also ping a core maintainer for review before merging PRs.

oneraghavan pushed a commit to oneraghavan/transformers that referenced this pull request Sep 26, 2022
* return post-processed segmentations as list, add test
* use torch to resize logits
* fix assertion error if no target_size is specified
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