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

cuda string incompatibility #2270

Open
1 task
erykoff opened this issue Dec 2, 2021 · 22 comments
Open
1 task

cuda string incompatibility #2270

erykoff opened this issue Dec 2, 2021 · 22 comments

Comments

@erykoff
Copy link
Contributor

erykoff commented Dec 2, 2021

Issue:

Building today in CI I am getting a cuda string error:

  File "/opt/conda/lib/python3.8/site-packages/conda/models/match_spec.py", line 763, in merge
    raise ValueError("Incompatible component merge:\n  - %r\n  - %r"
ValueError: Incompatible component merge:
  - '*cuda'
  - '*_cuda'

Example CI build here: https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=421595&view=logs&j=517fe804-fa30-5dc2-1413-330699242c05&t=c10fa5f2-fdf6-5338-3bdb-c4bea7c23412

from this PR: conda-forge/root-feedstock#151

This was rerendered with the following commit: "MNT: Re-rendered with conda-build 3.21.4, conda-smithy 3.15.0, and conda-forge-pinning 2021.12.02.17.12.32"

cc @beckermr


Environment (conda list):
$ conda list


Details about conda and system ( conda info ):
$ conda info

@beckermr
Copy link
Member

beckermr commented Dec 2, 2021

cc @conda-forge/core any ideas on this one?

@erykoff
Copy link
Contributor Author

erykoff commented Dec 2, 2021

@isuruf
Copy link
Member

isuruf commented Dec 2, 2021

@h-vetinari
Copy link
Member

What I don't understand is how faiss (much less tsnecuda, which is brandnew) would affect the root build?

@h-vetinari
Copy link
Member

Nevermind, just saw conda-forge/conda-forge-repodata-patches-feedstock#194.

@erykoff
Copy link
Contributor Author

erykoff commented Dec 2, 2021

This is a failure in the final packaging step, so I don't know where the dependency tree is coming from. But I put in some debug statements and it's definitely the faiss/tsnecuda constraint conflict.

But I believe I can close this as it's being addressed in conda-forge/conda-forge-repodata-patches-feedstock#194 and conda-forge/tsnecuda-feedstock#5

@erykoff erykoff closed this as completed Dec 2, 2021
@h-vetinari
Copy link
Member

Well, beyond the fact that it happened now, we should maybe use this issue to discuss how to avoid it in the future?

It was a simple oversight to use *cuda now vs. *_cuda a year ago, not sure if someone will trip over the same wire, but it seems possible?

Also, given the fall-out (of affecting global resolution in completely unrelated feedstocks), I'm now worried to touch this at all (e.g. remove the underscore to follow a uniform approach, if that's what we think would be best).

@beckermr
Copy link
Member

beckermr commented Dec 2, 2021

yeah that this broke the entire thing is kind of nuts, right?

Does that mean there is a bug in conda/mamba somewhere?

@erykoff
Copy link
Contributor Author

erykoff commented Dec 2, 2021

This showed up in conda build, completely independent of mamba, fwiw.

But the fact that it broke everything is nuts, and also the error message is ... less detailed than it could be.

@jakirkham
Copy link
Member

Meaning mamba worked and conda-build didn't? Or they both didn't work?

@erykoff
Copy link
Contributor Author

erykoff commented Dec 2, 2021

Meaning conda build crashed in this strange way, and I didn't use mambabuild because I don't know if it works with multi-outputs yet?

@jakirkham
Copy link
Member

Ok. Was wondering because if they have different behaviors that might help in isolating the bug.

@jaimergp
Copy link
Member

jaimergp commented Dec 3, 2021

Ugh, I have seen that error in recipes that merge build and host, since if a package name is present in both envs, the actual dist needs to be exactly the same. But that recipe doesn't do that so why is conda-build trying to merge specs like that? 🤷

@beckermr
Copy link
Member

beckermr commented Dec 3, 2021

Yeah exactly! That seems like a bug.

@beckermr
Copy link
Member

beckermr commented Dec 3, 2021

I don't think any of us are following. Where in conda build is this merging done?

@isuruf
Copy link
Member

isuruf commented Dec 3, 2021

It's not a conda-build issue. It's conda. See conda-forge/tsnecuda-feedstock#4 (comment)

@erykoff
Copy link
Contributor Author

erykoff commented Dec 3, 2021

Right, it's in the "packaging" phase of the build process:

2021-12-02T20:52:39.0771535Z Packaging root
2021-12-02T20:52:39.0779541Z INFO:conda_build.build:Packaging root
2021-12-02T20:52:39.0780765Z INFO conda_build.build:build(2289): Packaging root
2021-12-02T20:52:48.4227175Z Collecting package metadata (repodata.json): ...working... done
2021-12-02T20:53:21.2465730Z Solving environment: ...working... failed

@jaimergp
Copy link
Member

jaimergp commented Dec 3, 2021

Hm, it's called by Resolve.get_conflicting_specs(), used by conda's pre-checks before calling the SAT solver (it tries to see if a spec requested in the CLI will conflict with things already installed, so these are not pinned in the first attempt). While the solver tries to generate the clauses, it will request a MatchSpec for the installed PackageRecord, and this is where PackageRecord.combined_depends ends up calling MatchSpec.merge. If any of the components does not match, we get that error :/

Mamba does not use that afaik, so if mambabuild also patches that phase, it should be ok there?

@erykoff
Copy link
Contributor Author

erykoff commented Dec 3, 2021

Just to be clear, this is happening in a global sense? Because the package here doesn't depend on libfaiss at all.

@h-vetinari
Copy link
Member

@erykoff, could you reopen this? I think we still haven't followed the underlying issue to the end, and the context of this thread is perhaps better than a new issue?

@erykoff erykoff reopened this Dec 10, 2021
@jaimergp
Copy link
Member

This issue will go away if conda/conda#11612 is merged. However, the general solution for "compatible globs" is not trivial and requires regex look-aheads.

Btw, that PR taught me that conda indeed supports full regex queries o.o

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants