-
Notifications
You must be signed in to change notification settings - Fork 332
[Autotuner] Improve python package management #2859
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
Conversation
@@ -41,7 +41,9 @@ def load_dir(dir: str) -> pd.DataFrame: | |||
params = [] | |||
failed = [] | |||
for params_fname in glob.glob(f"{dir}/*/params.json"): | |||
metrics_fname = params_fname.replace("params.json", "metrics.json") | |||
metrics_fname = params_fname.replace("params.json", "metrics.json").replace( | |||
"ray", "or-0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to replace ray
with or-0
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In latest Ray versions this is an example file structure (or
is replaced with or-0
)
autotuner-best-5dfabe02.json searcher-state-2025-02-18_17-10-34.pkl
experiment_state-2025-02-18_17-10-34.json variant-AutoTunerBase-5dfabe02-or-0
search_gen_state-2025-02-18_17-10-34.json variant-AutoTunerBase-5dfabe02-ray
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -232,6 +232,11 @@ python3 ./tools/AutoTuner/test/smoke_test_tune.py | |||
python3 ./tools/AutoTuner/test/smoke_test_sample_iteration.py | |||
``` | |||
|
|||
## Developer reference | |||
|
|||
1. The [`uv`](https://docs.astral.sh/uv) package manager is used for handling different package requirements in AutoTuner. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What problem uv
resolves that pip
or venv
do not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Fastest python package manager/resolver
- It also acts as a catch-all for
pip
,venv
and additionallypip-tools
- which can manage python dependencies across different operating systems and versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pip
and venv
are cross OS, right? And if we are committing the requirements file, this feature of supporting multiple versions (of Python?) will be lost.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The committed requirements file actually is the solved deps for these versions, if not an error will be reported during uv compilation.
tools/AutoTuner/Makefile
Outdated
@uv pip compile --output-file=requirements.txt pyproject.toml --upgrade | ||
@uv pip compile --output-file=requirements-dev.txt pyproject.toml --extra dev --upgrade |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we are generating the files, why keep them in git?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For faster dependency resolving. The idea is to first store the solved dependencies in uv.lock
and requirements*.txt
file and next time, we can just install directly from these files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also has a side benefit of locking the python versions installed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am still not sold on this. What is the process exactly? We update pyproject.toml
, then use uv
to generate uv.lock
and requirements.txt
, and finally use pip
to install the requirements.txt
? Why not just write requirements.txt
? This looks like many extra steps that need to be tracked manually.
If we are using pip to install the dependencies at the end, looks to me that all the extra info that uv
saves in the lock file is moot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please provide an example of how the files are generated, in order, and used.
Also, provide another example to showcase a situation where having uv
will be beneficial.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea i was aiming for is to periodically run the uv requirements updater (ci or manually) and if it passes test, merge the updated requirements. This step is supposed to be dev-only, so users need not worry.
You are right that uv.lock file is not used in production - that is just to minimise complexity in Dockerfile. We use vanilla pip install on the compiled requirements.txt file.
edit: No other benefits for now, since we are not using uv.lock in production
Signed-off-by: Jack Luar <[email protected]>
Signed-off-by: Jack Luar <[email protected]>
…ards Signed-off-by: Jack Luar <[email protected]>
…p/ray) Signed-off-by: Jack Luar <[email protected]>
…tmp instead of local) Signed-off-by: Jack Luar <[email protected]>
…ctly Signed-off-by: Jack Luar <[email protected]>
Signed-off-by: Jack Luar <[email protected]>
Signed-off-by: Jack Luar <[email protected]>
Signed-off-by: Jack Luar <[email protected]>
… reliably - currently under gp/dp constraint -> 0.625 of the configurations are valid. if we use 5 samples -> only 0.625*0.625*5 ~ 1.9 samples will be valid. - if we use 8 samples ~ 3.1 samples will be valid. Signed-off-by: Jack Luar <[email protected]>
Signed-off-by: Jack Luar <[email protected]>
Signed-off-by: Jack Luar <[email protected]>
d8a7266
to
bed43c8
Compare
uv
, a fast rust based package manager for python.Fixes #2888