Skip to content

Conversation

@wirthual
Copy link
Collaborator

Related Issue

#590

Additional Notes

Check if transformers package is < 4.49 and if not, set flag to disable bettertransformers.

@wirthual wirthual requested review from Copilot and michaelfeil June 15, 2025 21:05
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

Added version compatibility checking for transformers package in BetterTransformer integration to prevent runtime crashes with newer versions.

  • Added version check in libs/infinity_emb/infinity_emb/transformer/acceleration.py to disable BetterTransformer when transformers package ≥ 4.49.0
  • Implements graceful fallback behavior instead of crashing when incompatible versions are detected

1 file reviewed, 1 comment
Edit PR Review Bot Settings | Greptile

Comment on lines 11 to 21
if CHECK_OPTIMUM.is_available:
from optimum.bettertransformer import ( # type: ignore[import-untyped]
BetterTransformer,
BetterTransformerManager,
)
transformers_version_string = version('transformers')
transformers_version = tuple([int(number) for number in transformers_version_string.split(".")])
if transformers_version < (4,49,0):
bettertransformer_available = True
from optimum.bettertransformer import ( # type: ignore[import-untyped]
BetterTransformer,
BetterTransformerManager,
)
else:
bettertransformer_available = False
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: Make bettertransformer_available global before usage. If CHECK_OPTIMUM.is_available is False, this variable will be undefined when used later.

Suggested change
if CHECK_OPTIMUM.is_available:
from optimum.bettertransformer import ( # type: ignore[import-untyped]
BetterTransformer,
BetterTransformerManager,
)
transformers_version_string = version('transformers')
transformers_version = tuple([int(number) for number in transformers_version_string.split(".")])
if transformers_version < (4,49,0):
bettertransformer_available = True
from optimum.bettertransformer import ( # type: ignore[import-untyped]
BetterTransformer,
BetterTransformerManager,
)
else:
bettertransformer_available = False
bettertransformer_available = False
if CHECK_OPTIMUM.is_available:
transformers_version_string = version('transformers')
transformers_version = tuple([int(number) for number in transformers_version_string.split(".")])
if transformers_version < (4,49,0):
bettertransformer_available = True
from optimum.bettertransformer import ( # type: ignore[import-untyped]
BetterTransformer,
BetterTransformerManager,
)
else:
bettertransformer_available = False

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds a runtime check for the installed Transformers version to disable BetterTransformer when the version is 4.49.0 or higher.

  • Import and parse transformers version
  • Conditionally set bettertransformer_available and import BetterTransformer
  • Early-return in relevant functions if BetterTransformer is unavailable
Comments suppressed due to low confidence (3)

libs/infinity_emb/infinity_emb/transformer/acceleration.py:65

  • [nitpick] The warning could be more informative by including the detected transformers version in the message to aid debugging.
logger.warning(

libs/infinity_emb/infinity_emb/transformer/acceleration.py:13

  • The new version-checking logic isn't covered by existing tests. Consider adding unit tests for scenarios below, at, and above the 4.49.0 threshold.
transformers_version = tuple([int(number) for number in transformers_version_string.split(".")])

libs/infinity_emb/infinity_emb/transformer/acceleration.py:9

  • If CHECK_OPTIMUM.is_available is false, bettertransformer_available will never be defined, causing a NameError. Initialize bettertransformer_available = False before this block.
from importlib.metadata import version

BetterTransformer,
BetterTransformerManager,
)
transformers_version_string = version('transformers')
Copy link

Copilot AI Jun 15, 2025

Choose a reason for hiding this comment

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

Parsing the version string by splitting and int-casting can fail on pre-release or metadata tags. Consider using packaging.version.parse for robust comparisons.

Copilot uses AI. Check for mistakes.
@codecov-commenter
Copy link

codecov-commenter commented Jun 15, 2025

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 75.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.34%. Comparing base (3286e76) to head (5647cab).
⚠️ Report is 20 commits behind head on main.

Files with missing lines Patch % Lines
...inity_emb/infinity_emb/transformer/acceleration.py 75.00% 3 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #600      +/-   ##
==========================================
- Coverage   79.85%   79.34%   -0.51%     
==========================================
  Files          43       43              
  Lines        3489     3500      +11     
==========================================
- Hits         2786     2777       -9     
- Misses        703      723      +20     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@wirthual
Copy link
Collaborator Author

Fails for version strings e.g. 4.56.0.dev0 which would require additional dependency like this

Consider #619 as replacement.

@wirthual wirthual closed this Aug 27, 2025
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.

2 participants