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

Changed solver behaviour in release 1.4.2 #2486

Closed
2 tasks done
jaimergp opened this issue Apr 24, 2023 · 9 comments
Closed
2 tasks done

Changed solver behaviour in release 1.4.2 #2486

jaimergp opened this issue Apr 24, 2023 · 9 comments
Assignees
Labels
type::bug Something isn't working where::libsolv

Comments

@jaimergp
Copy link
Contributor

Troubleshooting docs

  • My problem is not solved in the Troubleshooting docs

How did you install Mamba?

Other (please describe)

Search tried in issue tracker

N/A

Latest version of Mamba

  • My problem is not solved with the latest version

Tried in Conda?

Not applicable

Describe your issue

The new 1.4.2 release broke some conda integration tests that used to work with all previous versions. I took a look at the changelog and I guess the breaking change must be in one of these three PRs:

We are keeping a list of broken tests at conda/conda-libmamba-solver#186. I'll add more details as we investigate, but the key point is that downgrading to 1.4.1 makes everything pass.

mamba info / micromamba info

N/A

Logs

N/A

environment.yml

N/A

~/.condarc

N/A
@jaimergp
Copy link
Contributor Author

@AntoinePrv
Copy link
Member

AntoinePrv commented Apr 25, 2023

Hi @jaimergp, thanks for reporting this!

Do you know what is the changed behavior? For instance "when updating with a channel-specific ms, previous libmamba would get [...], now it gets [...] / report an error".

If the change is arbitrary, would you consider that this could be an acceptable break for a libmamba 2.0?

@costrouc
Copy link

costrouc commented May 27, 2023

@AntoinePrv so I did a deep dive into this issue and have all the details in conda/conda-libmamba-solver#186 (comment) onwards.

I have three branches

Good branch

costrouc@68c19e5 which just adds some printing of the libsolv state m_flags and m_jobs and what the matchspec <-> id is and is based on the commit just prior to 0345b9d (the commit which "breaks" a few conda-libmamba-solver tests.

Bad branch

costrouc@99ac04e which again just adds some printing of the libsolve state m_flags and m_jobs and what the matchspec <-> id is and is based on the "bad" commit 0345b9d

"Fix" Branch

This is a branch which has a single commit which "fixes" the bad branch costrouc@f72a63d. Essentially this change throws away the conda channel in the matchspec when SOLVER_INSTALL is issued with https://github.com/costrouc/mamba/blob/issue-186-hack-fix/libmamba/src/core/solver.cpp#L308-L320 for the update condition?

This comment https://github.com/costrouc/mamba/blob/issue-186-hack-fix/libmamba/src/core/pool.cpp#L227-L229 does make me think those that this is all related.

The comments in issue conda/conda-libmamba-solver#186 (comment) dive deep into the libsolve state m_job and the integers.

cc @jaimergp since I now have a "fix" but it does feel wrong to throw away the channel.

@costrouc
Copy link

Also wanted to note here the scope of the issue. Looking at costrouc@f72a63d which "fixes" these failing tests in conda-libmamba-solver.

This breakage only occurs when performing an update and "complex" matchspecs are specified e.g. not just package name and version constraint (example conda-forge::numpy would be complex). So the scope is not too large.

From this work though it is clear that the mamba behavior did change for this condition (update + complex matchspecs).

@AntoinePrv
Copy link
Member

AntoinePrv commented May 30, 2023

@costrouc thanks for the details. #2433 did make a common entry point for resolving all matchspecs, which sparsingly done before.

if I understand correctly the behavior is with update commands only? I do remember changing the code used by update, but we did not have a clear semantic of what a channel-specific update means.
For instance:

micromamba create -n foo defaults::numpy     # 1
micromamba update -n foo conda-forge::numpy  # 2

Here what is expected of # 2?

  • A/ Update numpy to instead use the conda-forge channel?
  • B/ Update numpy only if it comes from the conda-forge channel?

The simple thing to do is B because it is simply evaluating the matchspec.

@jaimergp
Copy link
Contributor Author

@AntoinePrv I would expect more of A than B, but the difference is subtle depending on what versions are available on each channel.

  • Let's say we have defaults::numpy=1.10 and conda-forge::numpy=1.12, then the behaviour is obvious: use numpy=1.12 from conda-forge
  • If we have defaults::numpy=1.10 and conda-forge::numpy=1.10, I'd still say we should use conda-forge's numpy
  • However, if what we have is defaults::numpy=1.10 and conda-forge::numpy=1.9, we would need to consider what the user is expecting. A forced downgrade via changing channels? I think the channel::pkg_name syntax is really trying to say "I want this package from this channel" more strongly than "with the latest version". Otherwise the user would use conda update -c conda-forge numpy maybe? It's not clear and also not standardized. I am not sure either what conda classic is doing in these cases. It looks like channel has higher precedence than version? In that case, superseding the package from the new channel might be right thing to do?

@AntoinePrv
Copy link
Member

With more time to think about this, I think B/ is wrong. B/ implies that spec the user provided is applied to the installed package. However we already have update numpy=1.0 meaning update (or downgrade) to a numpy 1.0.
So the correct way to think about it is that the new package needs to match the spec (maybe that was clear for everyone else ^^).

@AntoinePrv
Copy link
Member

Would reverting the behavior back to A/ fix this issue?

@AntoinePrv
Copy link
Member

Closing as stale, especially with all the changes coming in 2.0. Feel free to reopen a new issue as needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type::bug Something isn't working where::libsolv
Projects
None yet
Development

No branches or pull requests

3 participants