Conversation
Signed-off-by: Andy Xie <andy.xning@gmail.com>
|
It should be fixed by #31764 already |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses a CI failure by pinning the timm package to version 1.0.22. The fix is implemented by adding a pip install command directly into two separate Buildkite CI configuration files. While this resolves the immediate import error, it introduces a maintenance issue by hardcoding and duplicating the dependency version across CI scripts. My review recommends centralizing this dependency management within the project's requirements files for better long-term maintainability.
| no_gpu: true | ||
| commands: | ||
| - "pip install git+https://github.com/TIGER-AI-Lab/Mantis.git || echo 'Mantis installation skipped (decord not available on CPU-only environment)'" | ||
| - pip install timm==1.0.22 |
There was a problem hiding this comment.
While pinning timm==1.0.22 correctly fixes the import error, installing it directly in the CI script is not ideal for long-term maintainability, especially since this change is duplicated in another file (.buildkite/test_areas/models_multimodal.yaml).
A better approach would be to manage this dependency in a centralized requirements file. Please consider updating requirements/test.in to specify timm==1.0.22, and then regenerate requirements/test.txt. This will ensure consistency and make future dependency management easier.
| no_gpu: true | ||
| commands: | ||
| - pip install git+https://github.com/TIGER-AI-Lab/Mantis.git | ||
| - pip install timm==1.0.22 |
There was a problem hiding this comment.
As mentioned in the comment for .buildkite/test-pipeline.yaml, hardcoding the timm version here creates duplication and a maintenance burden. It would be best to consolidate this dependency pin into the requirements/test.in file and regenerate the corresponding requirements.txt.
2 participants
Purpose
Fix ci failure in #29493. It seems that timm 1.0.23 version has a bug so we need to pin 1.0.22 version as a workaround.
Test Plan
ci
Test Result
ci pass.
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.