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

Use conda-lock for reproducible conda env #35986

Merged
merged 41 commits into from
Jan 14, 2024
Merged

Conversation

tobiasdiez
Copy link
Contributor

@tobiasdiez tobiasdiez commented Jul 24, 2023

The conda workflow is broken since a few months due to various issues that arise from some packages being updated in conda-forge and sage not being quite compatible with the new version. For example, currently it fails as Cython 3.0.3 is used.

To prevent such issues happening in the feature, we use conda-lock to create a lock file for each os + python combination, from which one can then create a reproducible conda env. These lock files are created using commands of the form:

conda-lock --channel conda-forge --kind env --platform linux-64 --platform osx-64 --file ./src/environment-dev-3.10.yml --filename-template "src/environment-dev-3.10-{platform}"

Follow-up: Add a github workflow that updates these lock files automatically, similar to:

📝 Checklist

  • The title is concise, informative, and self-explanatory.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation accordingly.

⌛ Dependencies

@tobiasdiez tobiasdiez added s: needs review s: run conda ci Run the conda workflow on this PR. labels Jul 24, 2023
src/pyproject.toml.m4 Outdated Show resolved Hide resolved
@mkoeppe
Copy link
Member

mkoeppe commented Jul 24, 2023

How can conda-lock (and in particular an automatic workflow that runs conda-lock and commits the lock files) be able to resolve to a working set of versions, when the problem is that the resolution does not work for users when they install from our environment?

Is it not just that some version constraints were missing (which you added here)?

@tobiasdiez
Copy link
Contributor Author

tobiasdiez commented Jul 24, 2023

How can conda-lock (and in particular an automatic workflow that runs conda-lock and commits the lock files) be able to resolve to a working set of versions, when the problem is that the resolution does not work for users when they install from our environment?

Is it not just that some version constraints were missing (which you added here)?

The point is that you always have a working environment based off the lock files. If the automatic update of the lock file fails, manual work is needed (e.g. adding a version constraint). But only then.

@mkoeppe
Copy link
Member

mkoeppe commented Jul 24, 2023

Could you explain the changes to bootstrap-conda?

Do I read it correctly that unversioned conda environment files are no longer generated?

@tobiasdiez
Copy link
Contributor Author

Could you explain the changes to bootstrap-conda?

Do I read it correctly that unversioned conda environment files are no longer generated?

Conda-lock needs a python=3.x line in the environment file to know what kind of python version to use. Thus, the bootstrap conda file now generates a bunch of enviroment files that are the same as the existing ones, except that they contain a python version specifier. All of them are git-ignored.

@tobiasdiez tobiasdiez added s: run conda ci Run the conda workflow on this PR. and removed s: run conda ci Run the conda workflow on this PR. labels Jul 24, 2023
@tobiasdiez tobiasdiez added s: run conda ci Run the conda workflow on this PR. and removed s: run conda ci Run the conda workflow on this PR. labels Jul 25, 2023
@maresb
Copy link

maresb commented Jul 29, 2023

Hey, as a fan of sage and maintainer of conda-lock, I'm very happy you're finding it useful.

Could you please test again with removing --no-mamba? We hope that the issues mentioned above were resolved with the release of Conda 23.7 a few days ago.

@tobiasdiez
Copy link
Contributor Author

@maresb For me removing the --no-mamba switch worked even with the slightly older versions of mamba 1.4.9 and conda 23.5.2. And I've used it to create the lock files in the last commit. Based on this experience conda/conda-lock#292 could be closed.

@mkoeppe @isuruf Anything else to do here?

src/doc/en/installation/conda.rst Show resolved Hide resolved
src/pyproject.toml.m4 Show resolved Hide resolved
@mkoeppe
Copy link
Member

mkoeppe commented Sep 27, 2023

Work items:

@tobiasdiez
Copy link
Contributor Author

All tests are passing now, so this should be ready to go.

@tobiasdiez
Copy link
Contributor Author

@dimpase are you okay with these changes here?

@dimpase
Copy link
Member

dimpase commented Dec 20, 2023

would you like to rebase it?

Copy link
Contributor Author

@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

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

Okay, done.

Copy link
Member

@dimpase dimpase left a comment

Choose a reason for hiding this comment

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

OK

@vbraun
Copy link
Member

vbraun commented Dec 22, 2023

merge conflict

@tobiasdiez
Copy link
Contributor Author

Thanks Dima for the review. Merge was trivial, so I'll keep the "positive review" label.

Copy link

Documentation preview for this PR (built with commit bc91e96; changes) is ready! 🎉

vbraun pushed a commit to vbraun/sage that referenced this pull request Dec 27, 2023
    
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->

<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->

The conda workflow is broken since a few months due to various issues
that arise from some packages being updated in conda-forge and sage not
being quite compatible with the new version. For example, currently it
fails as Cython 3.0.3 is used.

To prevent such issues happening in the feature, we use [conda-
lock](https://conda.github.io/conda-lock/) to create a lock file for
each os + python combination, from which one can then create a
reproducible conda env. These lock files are created using commands of
the form:
```bash
conda-lock --channel conda-forge --kind env --platform linux-64
--platform osx-64 --file ./src/environment-dev-3.10.yml --filename-
template "src/environment-dev-3.10-{platform}"
```


Follow-up: Add a github workflow that updates these lock files
automatically, similar to:
- https://github.com/ibis-
project/ibis/blob/master/.github/workflows/conda-lock.yml
- https://github.com/svopper/munaiah-
analyser/blob/main/out/SciTools/iris/.github_workflows_refresh-
lockfiles.yml
- https://github.com/chipsalliance/fpga-interchange-
tests/blob/main/.github/workflows/update_conda_lock.yml
- https://github.com/ESMValGroup/ESMValTool/blob/main/.github/workflows/
create-condalock-file.yml

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
- [ ] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.

### ⌛ Dependencies

- sagemath#36405: to get env files for each
python version
- sagemath#36767

<!-- List all open PRs that this PR logically depends on
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#35986
Reported by: Tobias Diez
Reviewer(s): Dima Pasechnik, Isuru Fernando, Matthias Köppe, Tobias Diez
vbraun pushed a commit to vbraun/sage that referenced this pull request Jan 2, 2024
    
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->

<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->

The conda workflow is broken since a few months due to various issues
that arise from some packages being updated in conda-forge and sage not
being quite compatible with the new version. For example, currently it
fails as Cython 3.0.3 is used.

To prevent such issues happening in the feature, we use [conda-
lock](https://conda.github.io/conda-lock/) to create a lock file for
each os + python combination, from which one can then create a
reproducible conda env. These lock files are created using commands of
the form:
```bash
conda-lock --channel conda-forge --kind env --platform linux-64
--platform osx-64 --file ./src/environment-dev-3.10.yml --filename-
template "src/environment-dev-3.10-{platform}"
```


Follow-up: Add a github workflow that updates these lock files
automatically, similar to:
- https://github.com/ibis-
project/ibis/blob/master/.github/workflows/conda-lock.yml
- https://github.com/svopper/munaiah-
analyser/blob/main/out/SciTools/iris/.github_workflows_refresh-
lockfiles.yml
- https://github.com/chipsalliance/fpga-interchange-
tests/blob/main/.github/workflows/update_conda_lock.yml
- https://github.com/ESMValGroup/ESMValTool/blob/main/.github/workflows/
create-condalock-file.yml

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
- [ ] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.

### ⌛ Dependencies

- sagemath#36405: to get env files for each
python version
- sagemath#36767

<!-- List all open PRs that this PR logically depends on
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#35986
Reported by: Tobias Diez
Reviewer(s): Dima Pasechnik, Isuru Fernando, Matthias Köppe, Tobias Diez
vbraun pushed a commit to vbraun/sage that referenced this pull request Jan 5, 2024
    
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->

<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->

The conda workflow is broken since a few months due to various issues
that arise from some packages being updated in conda-forge and sage not
being quite compatible with the new version. For example, currently it
fails as Cython 3.0.3 is used.

To prevent such issues happening in the feature, we use [conda-
lock](https://conda.github.io/conda-lock/) to create a lock file for
each os + python combination, from which one can then create a
reproducible conda env. These lock files are created using commands of
the form:
```bash
conda-lock --channel conda-forge --kind env --platform linux-64
--platform osx-64 --file ./src/environment-dev-3.10.yml --filename-
template "src/environment-dev-3.10-{platform}"
```


Follow-up: Add a github workflow that updates these lock files
automatically, similar to:
- https://github.com/ibis-
project/ibis/blob/master/.github/workflows/conda-lock.yml
- https://github.com/svopper/munaiah-
analyser/blob/main/out/SciTools/iris/.github_workflows_refresh-
lockfiles.yml
- https://github.com/chipsalliance/fpga-interchange-
tests/blob/main/.github/workflows/update_conda_lock.yml
- https://github.com/ESMValGroup/ESMValTool/blob/main/.github/workflows/
create-condalock-file.yml

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
- [ ] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.

### ⌛ Dependencies

- sagemath#36405: to get env files for each
python version
- sagemath#36767

<!-- List all open PRs that this PR logically depends on
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#35986
Reported by: Tobias Diez
Reviewer(s): Dima Pasechnik, Isuru Fernando, Matthias Köppe, Tobias Diez
@vbraun vbraun merged commit 2309098 into sagemath:develop Jan 14, 2024
21 checks passed
@mkoeppe mkoeppe added this to the sage-10.3 milestone Jan 14, 2024
@tobiasdiez
Copy link
Contributor Author

Could someone of the @sagemath/core members please change the required workflows in the branch protection rules to include all of the current 6 conda workflows and remove the ones with the old naming "ubuntu-latest". Thanks!

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

Successfully merging this pull request may close these issues.

None yet

6 participants