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

CEP 3 - Using the Mamba solver in conda. #2

Merged
merged 8 commits into from
Jun 9, 2022
Merged

CEP 3 - Using the Mamba solver in conda. #2

merged 8 commits into from
Jun 9, 2022

Conversation

jezdez
Copy link
Member

@jezdez jezdez commented Jul 22, 2021

This depends on #1 and is definitely the first pass only.

@jezdez jezdez changed the title Add initial draft for CEP 3. Add initial draft for CEP 3 - Using the Mamba solver in Conda. Jul 22, 2021
@jezdez jezdez changed the title Add initial draft for CEP 3 - Using the Mamba solver in Conda. CEP 3 - Using the Mamba solver in Conda. Jul 22, 2021
cep-3.md Outdated
Conda contains a solver registry in the `conda.resolve` module that
must be extended to use the plugin architecture specified in CEP 2.

Conda currently (4.10.x) supports three solvers:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One important differentiation as far as I understand mamba is also that the building of the SAT clauses is done in C++ and not in Python. This also saves a considerable amount of time and is different to the current interface in https://github.com/conda/conda/blob/248741a843e8ce9283fa94e6e4ec9c2fafeb76fd/conda/resolve.py#L44

We should still be able to provide a registry in the conda.resolve module, probably just on a different level than the current one.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this'll be part of the implementation of the CEP, I'll update the draft to include a note about the difference in API levels and a need to refactor and/or integrate appropriately. Thanks for raising this!

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xhochy do you have any data on this? The claim(? - as far as I understand) that the SAT clause definition is somehow a bottleneck seems a bit weird to me 😅

Admittedly, I am not super knowledgeable on SAT solvers, but I would not expect this to take for than a couple milliseconds.

Usually, the bottleneck on dependency resolvers that work on remote sources is fetching the metadata necessary for the dependency resolution. So, it usually tends to be better to give more weight to the resolution algorithm instead of this kind of optimizations 😄

I haven't looked too much in all the possible backends, I am just raising the question.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This meta issue has some useful links to understand the bottlenecks: conda/conda#7700

@jezdez
Copy link
Member Author

jezdez commented May 17, 2022

@conda-incubator/steering

This PR falls under the Enhancement Proposal Approval policy of the conda governance policy, please vote and/or comment on this PR.

This PR needs 60% of the Steering Council to vote yea to pass.

To vote please leave Approve (yea) or Request Changes (nay) pull request reviews.

If you would like changes to the current language please leave a comment (in the PR) or push to this branch.

This vote will end on 2022-05-24.

@jezdez jezdez changed the title CEP 3 - Using the Mamba solver in Conda. CEP 3 - Using the Mamba solver in conda. May 17, 2022
@jezdez
Copy link
Member Author

jezdez commented May 19, 2022

Hey all, this is just a reminder that another vote is happening in #23 Thank you!

Copy link

@dharhas dharhas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jezdez jezdez added the vote Voting following governance policy label May 31, 2022
@jezdez
Copy link
Member Author

jezdez commented Jun 9, 2022

Voting results

This was a standard, non-timed-out vote.

This vote has reached quorum (17 + 0 = 17 which is at least 60% of 21).

It has also passed since it recorded 18 "yes" votes and 0 "no" votes giving 18/18 which is greater than 60% of 18.

@jezdez jezdez merged commit 75ab4fc into main Jun 9, 2022
@jezdez jezdez deleted the cep-3-draft branch June 9, 2022 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
vote Voting following governance policy
Projects
None yet
Development

Successfully merging this pull request may close these issues.