-
Notifications
You must be signed in to change notification settings - Fork 13
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
feat!: decide on explicit requirements first #61
feat!: decide on explicit requirements first #61
Conversation
f6ce550
to
db73dec
Compare
db73dec
to
42488ea
Compare
I used the attached notebook to generate the following images to get a better idea what the performance impact of this method: This shows that when running on a large set of examples with single requirements there is no real difference with the code from This shows the differences in solve times for individual packages: I conclude that at least for solving for specific solvables there is no noticable performance difference. 👍 But it would be good to profile this when solving for multiple requirements at the same time which will likely have a larger impact. Ill try that later. |
I modified the benchmark program to select 1000 (deterministic) random set of requirements from the snapshot. I think this provides a better overview of what is going on. These results show that on average solving becomes slower and although there are some cases that become (significantly) faster overall the solver becomes slightly slower (and in some cases significantly so). I am a little unsure what to do. These changes also result in "better" outcomes where the root requirements are optimized. So I'm inclined to merge this. |
Hi @baszalmstra, Thank you for posting those results. Let me block some time this week so that I can review it. |
I have not dived into the implementation, I am just looking at the second results given in #61 (comment).
I am also unsure about what to do here. Functionally, this is interesting, but do we want to have this new behavior be the default if some users know regression? Could this be introduced as a new alternative strategy? |
Yeah, perhaps we can abstract the decision making logic and allow users to inject certain strategies. |
Taken from mamba-org#61 Co-authored-by: Bas Zalmstra <bas@prefix.dev> Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Taken from mamba-org#61 Co-authored-by: Bas Zalmstra <bas@prefix.dev> Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
We ran into this again today with pixi. If you solve for The reason this happens is that there are lots of pytorch variants available so the solver will decide on it very late. Before it decides on I am therefore inclined to fix-up this PR and merge it, even though the performance might degrade a little. I am quite confident that once we merge #37 the performance regression introduced here will no longer be an issue anyway. |
Up to you, @baszalmstra. I will come back to resolvo when I can. |
This bumps rattler to the latest version. The most notable change is: - mamba-org/resolvo#61
Fixes #60
An implementation of the ideas discussed in #60. This PR opts to decide on requirements from the root over any requirement that the root does not directly depend on. This has the effect that the highest versions of requirements from the root are most likely picked instead of them being constrained by transitive dependencies.
The
test_explicit_root_requirements
test shows this in action. With the code before this PR the test would result inwhereas now the result is:
Note that the root requirements are optimized at the cost of the transitive requirement.