More explicit logic for CONDA_LIBMAMBA_SOLVER_MAX_ATTEMPTS #394
Conversation
conda-bot
added
the
cla-signed
conda_libmamba_solver/solver.py
Outdated
| return max_attempts_from_env | ||
| if in_state.update_modifier.FREEZE_INSTALLED: | ||
| # this the default, but can be overriden with --update-specs | ||
| # TODO: should we cap this at a reasonable number? some base envs have 100s of pkgs |
There was a problem hiding this comment.
This is the main point of this PR :) Thoughts?
There was a problem hiding this comment.
I haven't messed with libmamba solver at all, so absolutely no idea/feeling about what ranges would be sensible.
If you don't want a hard limit, you could use some function that just increases slower for higher inputs.
If you want a hard limit you could either do min(n_installed, limit) or something that only approaches the limit gradually like math.ceil(n_installed / (1 + (n_installed / limit)**2)**.5) or something more/less sophisticated.
There was a problem hiding this comment.
Each solve is usually sub-second, but some complex one that need some extra backtracking (like the one reported in your issue) take a 2-4s. If you end up with 100 installed packages then you need to wait a few minutes for the solver to give up.
By "giving up" I mean that we stop trying to unlock installed packages and we let the solver modify them as needed, just as mamba does. So maybe we can get by with 10 attempts. In other words, let the solver unfreeze up to 5-10 conflicting installed packages one by one until we just let everything float.
Alternatively, we could make the retry loop be based on time spent and not iterations, but that might be more complex than necessary.
There was a problem hiding this comment.
I'm going to try with 10 attempts and see if that is enough to pass all tests. Then we adjust as necessary.
There was a problem hiding this comment.
Makes sense.
Could you add that as a constant variable (DEFAULT_LIBMAMBA_SOLVER_MAX_ATTEMPTS or something ) to better track that?
Alternatively, we could make the retry loop be based on time spent and not iterations, but that might be more complex than necessary.
Yes, complexity, but also determinism ;).
|
Errors are due to conda/conda#13360 |
| int(os.environ.get("CONDA_LIBMAMBA_SOLVER_MAX_ATTEMPTS", len(in_state.installed))) + 1, | ||
| ) | ||
| for attempt in range(1, max_attempts): | ||
| for attempt in range(1, self._max_attempts(in_state) + 1): |
There was a problem hiding this comment.
Would it be useful to prepopulate the max attempts at the start of the solve as a instance attribute, instead of inline, so it's easier to debug?
There was a problem hiding this comment.
I'm not sure if this might break some tests. It shouldn't, but by saving it as instance attribute we are assuming we are going to have the same number of attempts for every call to solve_for_*() (for the lifetime of the same instance). In some instances, that might be tied to len(in_state.installed).
So I'd rather keep it like it is because it's technically more correct and does not assume that "one instantiation, one solve".
No, gh-391 actually failed on the first attempt for me. gh-381 incidentally fixes the example from gh-391, but it is unclear if other cases might still fail depending on the ordering; see the discussing from #391 (comment) onward. |
Co-authored-by: Jannis Leidel <jannis@leidel.info>
Ah, thanks for the pointer, much appreciated! |
jezdez
changed the title
Description
Inspired by #391. Refactoring a bit before we delve in the question of "how many attempts should we try before giving up?".
cc @mbargull
Checklist - did you ...
newsdirectory (using the template) for the next release's release notes?