Venv creation and uv support #245
Open
+9,780
−283
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
matbun
reviewed
Nov 13, 2024
* Refactor Dockerfiles * Refactor container gen script * ADD jlab dockerfile * First working version of jlab container * ADD CMCC requirements * update dockerfiles * ADD nvconda and refactor * Update containers * ADD containers * ADD simple plus dockerfile * Update NV deps * Update CUDA * Add comment * Cleanup * Cleanup * UPDATE README * Refactor * Fix linter * Refactor dockerfiles and improve tests * Refactor * Refactor * Fix * Add first tests for HPC * First broken tests for HPC * Update tests and strategy * UPDATE tests * Update horovod tests * Update tests and jlab deps * Add MLFLow tracking URI * ADD distributed trainer tests * mpirun container deepspeed * Fix distributed strategy tests on multi-node * ADD srun launcher * Refactor jobscript * Cleanup * isort tests * Refactor scripts * Minor fixes * Add logging to file for all workers * Add jupyter base files * Add jupyter base files * spelling * Update provenance deps * Update DS version * Update prov docs * Cleanup * add nvidia dep * Remove incomplete work * update pyproject * ADD hadolit config file * FIX flag * Fix linters * Refactor * Update prov4ml * Update pytest CI * Minor fix * Incorporate feedback * Update Dockerfiles * Incorporate feedback * Update comments * Refactor tests
|
If this PR is merged after #249, we need to update the pyproject.toml to use the |
matbun
reviewed
Nov 26, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR updates the way we create venvs removing parts of the reliance on massive scripts such as
generic_torch.sh. It also allows us to useuv, which is quite a bit faster than just usingpip. Of course, this is done without depending onuv, as it is still possible to only usepip.I wrote a quick tutorial on how the
uvworkflow goes, which can be seen in theuv-tutorial.mdfile that was added here.Motivation
The reliance on the
generic_torch.shscript has numerous disadvantages. First of all, we are unable to provide all of our dependencies in thepyproject.toml, which means that a simplepip install itwinaiis simply not possible at the moment. Secondly, thegeneric_torch.shscript is messy with many if statement that are repeated multiple times throughout, such as "if cuda" (but in shell syntax), meaning that it is hard to build upon the script. Thirdly, because we have a bunch of separatedpip install xstatements, we effectively give no way for the dependency manager to solve our dependency graph in a nice way. This results in many packages being installed only to be uninstalled on the nextpip installstatement. This causes our script to take much longer than needed.Noteworthy
Because part of this PR is also about transitioning to
uv, I have renamed a lot of the venvs that we use to just.venv. It seems that this work better withuvin some cases. I was also thinking that we could keep our old venvs and just symlink them to.venv, so that we don't have to create new names if we ever have to change systems again. This should hopefully streamline our naming convention a bit more.Related issue :
#244