vision: PIL version check for enum change appears to break SIMD versions

šŸ› Describe the bug

This change appears to break current Pillow-SIMD version #5898

    if tuple(int(part) for part in PIL.__version__.split(".")) >= (9, 1):
  File "/home/.../lib/python3.10/site-packages/torchvision/transforms/_pil_constants.py", line 7, in <genexpr>
    if tuple(int(part) for part in PIL.__version__.split(".")) >= (9, 1):
ValueError: invalid literal for int() with base 10: 'post1'

Amusingly enough, I warned against this approach in a users PR in timm https://github.com/rwightman/pytorch-image-models/pull/1256

Would be nice to have it fixed before 1.12 is finalized, I just hit this trying out the RC

Versions

PT 1.12 RC, TV 0.13.0

About this issue

  • Original URL
  • State: closed
  • Created 2 years ago
  • Reactions: 1
  • Comments: 23 (15 by maintainers)

Most upvoted comments

@datumbox no strong reason, hasattr works, I think in this case there wouldn’t be any dir gotchas as it’s a enum in a module, hasattr does have stronger guarantees re attrib resolution in all cases though

@rwightman Thanks for reporting. Is there any specific reason you propose checking in dir() instead of doing hasattr(Image, "Resampling")?

@vfdev-5 Thanks for the ultra fast fix. I left this comment on the PR. I’ve tested with SIMD, PIL 9.0 and 9.1 and it works. Let me know what you think, thanks!

@vfdev-5 one last thing to keep in mind re the gap and future performance characterization, the Pillow-SIMD numbers are single threaded, I don’t believe it uses any thread parallelization, so if 8 threads are pinned for those tensor numbers, that is significant… esp when you’re often pinning 16-48 cores with pre-processing…

yup! you’ve been making great progress, I just need ALL the CPU munch munch 😃

cool thanks for the numbers, I measured Pillow-SIMD to be 8x faster than Pillow (I think that was bicubic) so would maybe put it ~ 3x faster than the CPU tensor numbers

@vfdev-5 yes, those basic ones for inference, but a wider array for train aug, solarize, posterize, other luts, rotate, skew, other geometric … but even just the resize is a big one, Pillow-SIMD is heavily optimized to do many / most? of the ops in uint8, last I looked I think many of the tensor ops necessitated using float which is a big hit.

I also try to move the GPU whenever I can before converting to float to keep bandwidth down.

it’s often underrated how fast Pillow-SIMD actually is, when you add the proper filtering to OpenCV (to prevent aliasing on downsample), Pillow-SIMD is usually faster than cv2 as well for typical pre-proc / aug pipelines. It’s just a shame it’s not portable or easy to integrate into most environments…

@vfdev-5 Pillow-SIMD was still significantly faster last time I checked (last fall I think). I’m often CPU bound in dataloader pre-processing so it has a big impact. Doing it on GPU eats precious GPU memory.

@adamjstewart If pytorch was bringing packaging as dep then we could use it, otherwise adding it into torchvision’s requirements should be carefully validated. Solution with dir works without warnings.

We should definitely use from packaging.version import Version instead of manual parsing:

from packaging.version import Version

v = Version("9.0.0.post1")
v > Version("9.1.0")
# False

However, the main drawback with packaging is that it is not a built-in package 😦