CI: Constrain all dependencies; introduce a Monday workflow to update pins#558
CI: Constrain all dependencies; introduce a Monday workflow to update pins#558mergify[bot] merged 4 commits intoinstructlab:mainfrom
Conversation
mergify
bot
added
CI/CD
booxter
force-pushed
the
constraints
branch
2 times, most recently
from
9cd5100 to
82c4916
Compare
|
The smoke failure is generic: #505 but I wonder how come we hit it twice here. |
|
@Mergifyio rebase |
✅ Branch has been successfully rebased |
nathan-weinberg
left a comment
nathan-weinberg
left a comment
There was a problem hiding this comment.
LGTM, will leave second approval to @instructlab/training-maintainers so they can indicate if they are happy with this direction
Signed-off-by: Ihar Hrachyshka <ihar.hrachyshka@gmail.com>
Signed-off-by: Ihar Hrachyshka <ihar.hrachyshka@gmail.com>
| source venv/bin/activate | ||
| # The list is taken from the pull request linked above | ||
| pip install torch packaging setuptools wheel psutil ninja | ||
| pip install torch packaging setuptools wheel psutil ninja -c constraints-dev.txt |
There was a problem hiding this comment.
I totally get that we have to do this step for flash-attn.
We have to use tox-current-env to enable this workaround, correct? i.e. because we can't build our venv in a single shot (what Tox supports) we instead have to do a four-part environment build:
- install Tox
- install the dependencies for flash-attention
- install tox-current-env
- run full package installation (including FA build and local package build)
There was a problem hiding this comment.
Yes. Is there some concern here? Not sure if the comment should be addressed in some form, or just acknowledged. :) Let me know!
| pip install . | ||
| pip_install="pip install -c constraints-dev.txt" | ||
| $pip_install -r ./deps.txt --no-build-isolation | ||
| $pip_install . |
There was a problem hiding this comment.
If we've installed the other project deps via $pip_install -r ./deps.txt --no-build-isolation do we want to do a --no-deps installation here?
There was a problem hiding this comment.
Not sure. I think the effect will be the same, since we indeed already pre-installed all dependencies, (with constraints enforced), but I'd rather not if it's not required. Do you think of some specific scenario where omitting --no-deps could bite us?
There was a problem hiding this comment.
I think effect is the same essentially
JamesKunstle
left a comment
JamesKunstle
left a comment
There was a problem hiding this comment.
Looks very good, mostly makes total sense to me. A few comments.
Signed-off-by: Ihar Hrachyshka <ihar.hrachyshka@gmail.com>
It will pull the `uv` based implementation that is a lot more speedy and robust. Signed-off-by: Ihar Hrachyshka <ihar.hrachyshka@gmail.com>
| pip install . | ||
| pip_install="pip install -c constraints-dev.txt" | ||
| $pip_install -r ./deps.txt --no-build-isolation | ||
| $pip_install . |
There was a problem hiding this comment.
I think effect is the same essentially
| # install with Torch and build dependencies installed | ||
| python3.11 -m pip install packaging wheel setuptools-scm | ||
| python3.11 -m pip install .[cuda] -r requirements-vllm-cuda.txt | ||
| PYTHON=python3.11 ./scripts/install-ilab-with-cuda.sh |
There was a problem hiding this comment.
nice sneaking this in here :) we needed to switch to this anyway
4 participants
Closes: #541