-
Notifications
You must be signed in to change notification settings - Fork 52
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
Pass CONDA_OVERRIDE_CUDA
to with_cuda
of conda-lock
#721
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -37,6 +37,18 @@ def print_cmd(cmd): | |
print_cmd(["conda", "config", "--show"]) | ||
print_cmd(["conda", "config", "--show-sources"]) | ||
|
||
# conda-lock ignores variables defined in the specification, so this code | ||
# gets the value of CONDA_OVERRIDE_CUDA and passes it to conda-lock via | ||
# the with_cuda parameter, see: | ||
# https://github.com/conda-incubator/conda-store/issues/719 | ||
# https://docs.conda.io/projects/conda/en/latest/user-guide/tasks/manage-virtual.html#overriding-detected-packages | ||
# TODO: Support all variables once upstream fixes are made to conda-lock, | ||
# see the discussion in issue 719. | ||
if specification.variables is not None: | ||
cuda_version = specification.variables.get("CONDA_OVERRIDE_CUDA") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The The example from the description of this PR ...
variables:
CONDA_OVERRIDE_CUDA: '12.0' Will come through as a string and be forwarded to conda-lock, but the example in the original issue has: ...
variables:
CONDA_OVERRIDE_CUDA: 11.8 Which would be parsed as float and be forwarded to conda-lock as a float. I don't see a test update associated with this, so I am assuming we don't have any coverage here, could you try this out locally to see if we need to convert the value as we pull it off the specification? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added a print to (In fact, the admin UI renders previously unquoted versions in quotes once you submit an env.) So no action is needed here. You also asked offline whether using
Please let me know if you have any other questions! |
||
else: | ||
cuda_version = None | ||
|
||
# CONDA_FLAGS is used by conda-lock in conda_solver.solve_specs_for_arch | ||
try: | ||
conda_flags_name = "CONDA_FLAGS" | ||
|
@@ -48,6 +60,7 @@ def print_cmd(cmd): | |
platforms=platforms, | ||
lockfile_path=lockfile_filename, | ||
conda_exe=conda_command, | ||
with_cuda=cuda_version, | ||
) | ||
finally: | ||
os.environ.pop(conda_flags_name, None) | ||
|
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.
Nit, so non blocking but since we said this is an "interim" solution it would be best to add also a TODO/something indicating that we should look at a more robust approach