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

Unexpected (?) behavior when torch is a sub dependency #9498

Closed
EgonFerri opened this issue Nov 28, 2024 · 2 comments · Fixed by #9370
Closed

Unexpected (?) behavior when torch is a sub dependency #9498

EgonFerri opened this issue Nov 28, 2024 · 2 comments · Fixed by #9370
Labels
bug Something isn't working

Comments

@EgonFerri
Copy link

EgonFerri commented Nov 28, 2024

I have this scenario:

I need a project that installs torch and torchvision ("torch==2.1.2", "torchvision==0.16.2") and a dependency (piq==0.8.0) that contains just one dependency: "torchvision>=0.10.0". I need to account for three different scenarios: Darwin OS (obv always without GPU) and Linux OS (both with and without GPU).

I am using the last version of uv (0.5.5).

[project]
name = "test"
version = "0.1.0"
description = "Test pytorch dependencies"
readme = "README.md"
requires-python = ">=3.11"
dependencies = ["piq==0.8.0"]

[project.optional-dependencies]
cpu = [
    "torch==2.1.2",
    "torchvision==0.16.2",
]
gpu = [
    "torch==2.1.2",
    "torchvision==0.16.2",
]

[tool]
[tool.uv]
conflicts = [
    [
        { extra = "cpu" },
        { extra = "gpu" },
    ],
]

[tool.uv.sources]
[[tool.uv.sources.torch]]
index = "pytorch-cpu"
marker = "platform_system != 'Darwin'"
extra = "cpu"

[[tool.uv.sources.torchvision]]
index = "pytorch-cpu"
marker = "platform_system != 'Darwin'"
extra = "cpu"

[[tool.uv.index]]
name = "pytorch-cpu"
url = "https://download.pytorch.org/whl/cpu"
explicit = true

Until the piq dependency enters the scene, everything works properly. But when I add piq (both if I add it to main dependencies or if I put it with CPU and GPU optional dependencies), it behaves unexpectedly adding both CPU and GPU versions of torch and torchvision (along with all heavy nvidia dependencies) even if i put --extra CPU:

$ uv sync
Using CPython 3.11.9 interpreter at: /usr/local/bin/python3
Creating virtual environment at: .venv
Resolved 34 packages in 1.[39](https://gitlab.pepita.io/core-aide/watergate/temp/-/jobs/3502801#L39)s
Audited in 0.01ms
$ uv sync --extra gpu
Resolved 34 packages in 2ms
Prepared 33 packages in 1m 02s
warning: Failed to hardlink files; falling back to full copy. This may lead to degraded performance.
         If the cache and target directories are on different filesystems, hardlinking may not be supported.
         If this is intentional, set `export UV_LINK_MODE=copy` or use `--link-mode=copy` to suppress this warning.
Installed 33 packages in 17.80s
 + certifi==2024.8.30
 + charset-normalizer==3.4.0
 + filelock==3.16.1
 + fsspec==2024.10.0
 + idna==3.10
 + jinja2==3.1.4
 + markupsafe==3.0.2
 + mpmath==1.3.0
 + networkx==3.4.2
 + numpy==2.1.3
 + nvidia-cublas-cu12==12.1.3.1
 + nvidia-cuda-cupti-cu12==12.1.105
 + nvidia-cuda-nvrtc-cu12==12.1.105
 + nvidia-cuda-runtime-cu12==12.1.105
 + nvidia-cudnn-cu12==8.9.2.26
 + nvidia-cufft-cu12==11.0.2.[54](https://gitlab.pepita.io/core-aide/watergate/temp/-/jobs/3502801#L54)
 + nvidia-curand-cu12==10.3.2.106
 + nvidia-cusolver-cu12==11.4.5.107
 + nvidia-cusparse-cu12==12.1.0.106
 + nvidia-nccl-cu12==2.18.1
 + nvidia-nvjitlink-cu12==12.6.85
 + nvidia-nvtx-cu12==12.1.105
 + pillow==11.0.0
 + piq==0.8.0
 + requests==2.32.3
 + sympy==1.13.3
 + torch==2.1.2
 + torch==2.1.2+cpu
 + torchvision==0.16.2
 + torchvision==0.16.2+cpu
 + triton==2.1.0
 + typing-extensions==4.12.2
 + urllib3==2.2.3
$ uv sync --extra cpu
Resolved 34 packages in 18ms
Uninstalled 4 packages in [70](https://gitlab.pepita.io/core-aide/watergate/temp/-/jobs/3502801#L70)7ms
warning: Failed to hardlink files; falling back to full copy. This may lead to degraded performance.
         If the cache and target directories are on different filesystems, hardlinking may not be supported.
         If this is intentional, set `export UV_LINK_MODE=copy` or use `--link-mode=copy` to suppress this warning.
Installed 4 packages in 5.23s
 ~ torch==2.1.2
 ~ torch==2.1.2+cpu
 ~ torchvision==0.16.2
 ~ torchvision==0.16.2+cpu
Cleaning up project directory and file based variables
00:01
Job succeeded

and when searching in the lock, it can be seen that piq has as dependency both torchvision and torchvision+cpu:

[[package]]
name = "piq"
version = "0.8.0"
source = { registry = "https://pypi.org/simple" }
dependencies = [
    { name = "torchvision", version = "0.16.2", source = { registry = "https://pypi.org/simple" } },
    { name = "torchvision", version = "0.16.2+cpu", source = { registry = "https://download.pytorch.org/whl/cpu" }, marker = "platform_system != 'Darwin'" },
]
sdist = { url = "https://files.pythonhosted.org/packages/8a/ae/9274f8e3f216759311a460a896594f04aade8a9f1171c0a732cc613c3b21/piq-0.8.0.tar.gz", hash = "sha256:ea74938ce11845d63fed3185694cdf9641f85bd19718dc3394f8008811b847d4", size = 91316 }
wheels = [
    { url = "https://files.pythonhosted.org/packages/71/49/e198c355bc01a1bb3aff5d41a8fe7c8a0502ad7a4063865d2c9dd270cb0f/piq-0.8.0-py3-none-any.whl", hash = "sha256:bad83f8e68bad6c09df09c2bdf50d1e8ccf5e852d7b64562317c27ed5eb63159", size = 106872 },
] 

Thanks in advance (:

@jenshnielsen
Copy link

I think this may be a duplicate of #9289 ?

@charliermarsh
Copy link
Member

That's right -- we're actively working on it. Feel free to follow the linked issue.

@charliermarsh charliermarsh closed this as not planned Won't fix, can't repro, duplicate, stale Nov 29, 2024
@charliermarsh charliermarsh added the bug Something isn't working label Nov 29, 2024
BurntSushi added a commit that referenced this issue Dec 10, 2024
This PR adds a notion of "conflict markers" to the lock file as an
attempt to address #9289. The idea is to encode a new kind of boolean
expression indicating how to choose dependencies based on which extras
are activated.

As an example of what conflict markers look like, consider one of the
cases
brought up in #9289, where `anyio` had unconditional dependencies on
two different versions of `idna`. Now, those are gated by markers, like
this:

```toml
        [[package]]
        name = "anyio"
        version = "4.3.0"
        source = { registry = "https://pypi.org/simple" }
        dependencies = [
            { name = "idna", version = "3.5", source = { registry = "https://pypi.org/simple" }, marker = "extra == 'extra-7-project-foo'" },
            { name = "idna", version = "3.6", source = { registry = "https://pypi.org/simple" }, marker = "extra == 'extra-7-project-bar' or extra != 'extra-7-project-foo'" },
            { name = "sniffio" },
        ]
```

The odd extra values like `extra-7-project-foo` are an encoding of not
just the conflicting extra (`foo`) but also the package it's declared
for (`project`). We need both bits of information because different
packages may have the same extra name, even if they are completely
unrelated. The `extra-` part is a prefix to distinguish it from groups
(which, in this case, would be encoded as `group-7-project-foo` if `foo`
were a dependency group). And the `7` part indicates the length of the
package name which makes it possible to parse out the package and extra
name from this encoding. (We don't actually utilize that property, but
it seems like good sense to do it in case we do need to extra
information from these markers.)

While this preserves PEP 508 compatibility at a surface level, it does
require utilizing this encoding scheme in order
to evaluate them when they're present (which only occurs when
conflicting extras/groups are declared).

My sense is that the most complex part of this change is not just adding
conflict markers, but their simplification. I tried to address this in
the code comments and commit messages.

Reviewers should look at this commit-by-commit.

Fixes #9289, Fixes #9546, Fixes #9640, Fixes #9622, Fixes #9498, Fixes
#9701, Fixes #9734
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants