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

Change requirements handling #475

Draft
wants to merge 29 commits into
base: main
Choose a base branch
from
Draft

Conversation

mschwoer
Copy link
Collaborator

@mschwoer mschwoer commented Feb 18, 2025

Changing handling of requirements:

@GeorgWa please check the changes to requirements.txt, maybe some constraints need to be re-added?

@mschwoer mschwoer requested a review from GeorgWa February 18, 2025 13:19
@mschwoer mschwoer mentioned this pull request Feb 18, 2025
[REQUIREMENTS] Update requirements.txt
@mschwoer mschwoer marked this pull request as ready for review February 18, 2025 14:11
xxhash==3.4.1
torchmetrics==1.4.0.post0
transformers==4.40.2
alphabase==1.5.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this be the requirements from the old requirements_loose.txt?
E.g. the minimal known constraints for feature compatability.

alpharaw>=0.3.1
alphatims>=1.0.8
alphabase>=1.5.0
peptdeep>=1.3.0
directlfq>=0.2.19
transformers<=4.40.2
numpy<2
scipy>=1.12.0

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

agree .. note that I dropped the constraint on transformers as we do support the latest version now

Comment on lines 53 to 65
nvidia-cublas-cu12==12.4.5.8
nvidia-cuda-cupti-cu12==12.4.127
nvidia-cuda-nvrtc-cu12==12.4.127
nvidia-cuda-runtime-cu12==12.4.127
nvidia-cudnn-cu12==9.1.0.70
nvidia-cufft-cu12==11.2.1.3
nvidia-curand-cu12==10.3.5.147
nvidia-cusolver-cu12==11.6.1.9
nvidia-cusparse-cu12==12.3.1.170
nvidia-cusparselt-cu12==0.6.2
nvidia-nccl-cu12==2.21.5
nvidia-nvjitlink-cu12==12.4.127
nvidia-nvtx-cu12==12.4.127
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note that these seem to be linux-specific, cf. https://github.com/MannLabs/alphadia/actions/runs/13411418109/job/37462368960#step:10:141
(same with triton)

We could add such dependencies to some "blacklist"?
Then the process would be:

  1. upgrade a dependency
  2. run freeze workflow
  3. run full test matrix
  4. in case of error: update blacklist and repeat from 2.?

@GeorgWa ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

basically, this would be a manually curated constraints.txt file,

nvidia-nvtx-cu12 ; sys_platform != 'linux'
...
triton ; sys_platform != 'linux'

which will be considered by the workflow when creating _requirements.freeze.txt

note that there might be other occasions for such a mechanism:
MannLabs/alphapeptdeep#227 (comment)

Copy link
Collaborator Author

@mschwoer mschwoer Feb 19, 2025

Choose a reason for hiding this comment

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

@mschwoer mschwoer marked this pull request as draft February 19, 2025 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants