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.

Sign up for GitHub

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 UDOP dtype issue #34180

Merged
merged 11 commits into from
Oct 18, 2024
Merged

Fix UDOP dtype issue #34180

Changes from 7 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
4441d40
Trigger UDOP tests
Rocketknight1 Oct 15, 2024
75aaa0a
Try forcing dtype in LayoutLMV3
Rocketknight1 Oct 15, 2024
0bb721e
Do checks to see where uint8 is getting in
Rocketknight1 Oct 15, 2024
242a680
Do checks to see where uint8 is getting in
Rocketknight1 Oct 15, 2024
3674df5
Found it!
Rocketknight1 Oct 15, 2024
f0fa8f8
Add .astype(np.float32)
Rocketknight1 Oct 15, 2024
d1f432f
Remove forced check, make fixup
Rocketknight1 Oct 15, 2024
4f855d3
Checking where exactly the uint8 creeps in
Rocketknight1 Oct 18, 2024
d7f0720
More checking on the uint8 issues
Rocketknight1 Oct 18, 2024
1c40330
Manually upcast in rescale()
Rocketknight1 Oct 18, 2024
8cfa5d0
Remove UDOP trigger
Rocketknight1 Oct 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/transformers/models/layoutlmv3/image_processing_layoutlmv3.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,9 @@ def preprocess(

if do_resize:
images = [
self.resize(image=image, size=size, resample=resample, input_data_format=input_data_format)
self.resize(image=image, size=size, resample=resample, input_data_format=input_data_format).astype(
np.float32
Copy link
Contributor

@NielsRogge NielsRogge Oct 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Shouldn't this be fixed in the modeling code? Would be inconsistent compared to all the other image processing classes we have

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately, I think the conversion to uint8 happens inside self.resize() (with some PIL versions), and then the error is thrown when we try to scale the data afterwards. It's quite hard for me to tell exactly where it's happening, though, because I can't reproduce the issue locally - it only occurs on the CI machine!

I'm not 100% sure but I think what happens is:

  1. data before resize is np.float32
  2. self.resize() creates a PIL.Image to do the resize
  3. On some PIL versions this converts data to uint8, on others it remains as float32
  4. The output dtype therefore depends on the PIL version

)
for image in images
]

Expand Down
Loading