Skip to content

Update environment pinning workflow to use conda-lock#1660

Merged
lkstrp merged 7 commits intoPyPSA:masterfrom
stefan-oet:feature/lockfile-for-dependencies
May 13, 2025
Merged

Update environment pinning workflow to use conda-lock#1660
lkstrp merged 7 commits intoPyPSA:masterfrom
stefan-oet:feature/lockfile-for-dependencies

Conversation

@stefan-oet
Copy link
Copy Markdown
Contributor

Changes proposed in this Pull Request

This PR improves our environment pinning workflow by replacing the previous multi-OS matrix approach with conda-lock.

Changes:

  • Replace separate OS runners with a single Ubuntu runner
  • Use conda-lock to generate lockfiles for all platforms (linux-64, osx-64, win-64)
  • Maintain the same file naming convention for backward compatibility
  • Improve dependency resolution and reproducibility across platforms

Checklist

  • I tested my contribution locally and it works as intended.
  • Code and workflow changes are sufficiently documented.
  • Changed dependencies are added to envs/environment.yaml.
  • Changes in configuration options are added in config/config.default.yaml.
  • Changes in configuration options are documented in doc/configtables/*.csv.
  • Sources of newly added data are documented in doc/data_sources.rst.
  • A release note doc/release_notes.rst is added.

@FabianHofmann FabianHofmann requested a review from lkstrp April 29, 2025 12:53
@@ -1,5 +1,4 @@
name: Update pinned envs

name: Update pinned envs with conda-lock
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
name: Update pinned envs with conda-lock
name: Update pinned envs

Can we keep that shorter?

Copy link
Copy Markdown
Member

@lkstrp lkstrp left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @stefan-oet !

I did not know about conda-lock. But adding build numbers without extra files for arm-based architecture will not work. Any silicon based macs can not resolve those, which is quite common. Linux ARM is probably less of an issue, but still.

We can either drop the build numbers entirely, or add environments for each architecture. I probably prefer the latter and then we can also break things and use new filenames (the defaults).

So could we do:


project/
├── envs/
│   ├── environment.yaml
│   ├── linux-64.lock.yaml
│   ├── osx-64.lock.yaml
│   ├── win-64.lock.yaml
│   ├── osx-arm64.lock.yaml
│   ├── linux-aarch64.lock.yaml

Any future rule specific environments can go into subdirs then.

And before we merge this, and when we change the filenames again, I'd like to see if we can add a uv lock as well. Not all dependencies can be installed via uv, but if the rest is already installed on the system, it would be such a convenience to have uv support.

@stefan-oet
Copy link
Copy Markdown
Contributor Author

Thanks for your quick response @lkstrp !

I agree that we should definitely include arm-based architecture support even though this will break the current default filenames.

Regarding uv lock, I'm not sure if we should explicitly add this to the repository as it does not support conda yet as you mentioned.

One way to have an implicit uv support is to use pixi (i.e. pixi init --import envs/environment.yaml) which support conda as well as pypi (see here under Details regarding the PyPI integration).

@lkstrp
Copy link
Copy Markdown
Member

lkstrp commented Apr 30, 2025

What about just adding a requirements.in for uv next to environment.yaml? And the pinned version in requirements.txt via uv pip compile. This will not install all needed dependencies, but I think only glpk is missing then, which can be installed system wide. The rest is available via pypi. You could then just run pip sync envs/requirements.txt which would be very convenient. Just as a second option to conda.

What would be the advantage of pixi over this process? It looks like could be used as well as another alternative on top of conda.

@FabianHofmann
Copy link
Copy Markdown
Contributor

and we don't need glpk anymore as we have highs :)

@lkstrp lkstrp mentioned this pull request Apr 30, 2025
5 tasks
@lkstrp
Copy link
Copy Markdown
Member

lkstrp commented Apr 30, 2025

I moved the uv discussion to #1663. Glad to hear your opinion there @stefan-oet ! But regardless of this, we can switch to uv-lock here.
I think best structure would be the one below:

project/
├── envs/
│   ├── environment.yaml
│   └── locks/
│       ├── linux-64.lock.yaml
│       ├── osx-64.lock.yaml
│       ├── win-64.lock.yaml
│       ├── osx-arm64.lock.yaml
│       └── linux-aarch64.lock.yaml

@stefan-oet
Copy link
Copy Markdown
Contributor Author

#1663 looks good to me and it works as long as glpk is installed.

The proposed file structure makes sense as well. Perhaps we can call the directory containing the lockfiles lockfiles instead of locks?

Regarding the pixi discussion, it does not really make sense here since we have (or had?) only 1 system dependency. It's just a modern alternative to conda

@lkstrp
Copy link
Copy Markdown
Member

lkstrp commented Apr 30, 2025

The proposed file structure makes sense as well. Perhaps we can call the directory containing the lockfiles lockfiles instead of locks?

Not sure, mm env create -f envs/locks/linux-64.lock.yaml is already quite long. If we add the pyproject.toml we can also stick to conda only environment files in ./envs again, which makes me again more leaning towards dropping the subdir altogether.

Another thing is that the created file no longer has a name. So you always have to pass -n. Would it be more convenient to keep pypsa-eur as the default env name?

@stefan-oet
Copy link
Copy Markdown
Contributor Author

I do agree that dropping the subdir makes more sense now.

Another thing is that the created file no longer has a name. So you always have to pass -n. Would it be more convenient to keep pypsa-eur as the default env name?

Fair point! I'll update the PR tomorrow

@stefan-oet
Copy link
Copy Markdown
Contributor Author

I had to do some digging this morning as it turned out that adding support for linux-aarch64 was not as obvious as that for osx-arm64. I remember having this same issue a few weeks ago but didn't spend any time looking into it at that time.

It turns out that conda / conda-lock will always resolve the conda dependencies first before moving to those defined under pip. And this can have implications sometimes. Like in our case we have highspy defined as a pip dependency and pypi does have support for highs on arm-based linux systems. However, pypsa depends on highs as well and since pypsa is a conda dependency in our environment.yaml, it will get resolved first. Unfortunately conda-forge does not have support for highs on arm-based linux systems so the dependency resolution fails. Defining pypsa as a pip dependency solves the problem here.

Note that this also explains why highspy does not end up as a pip dependency in our existing pinned yaml's. Same holds true for entsoe-py as it's already a dependency of powerplantmatching so it can be removed from the pip list in order to prevent potential future confusions.

Any objections against moving pypsa to the list of pip dependencies in order to have support for arm-based linux systems?

@lkstrp
Copy link
Copy Markdown
Member

lkstrp commented May 2, 2025

No objections that I know of. Just feels a bit wrong to further mix pip in there. We can also ask Highs if there is a reason for not providing linux arm on conda, and move PyPSA back when they do.

@stefan-oet
Copy link
Copy Markdown
Contributor Author

@lkstrp I've updated the PR. The HiGHS team has added support for ARM on linux 🙌

Copy link
Copy Markdown
Member

@lkstrp lkstrp left a comment

Choose a reason for hiding this comment

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

The HiGHS team has added support for ARM on linux 🙌

Lovely! I'll test the process once, and then it's ready to be merged.
One last thing: could you also add a release note for the changed file name?

@stefan-oet
Copy link
Copy Markdown
Contributor Author

One last thing: could you also add a release note for the changed file name?

I've added the missing lock files and have mentioned the breaking changes in the release notes.

@lkstrp lkstrp merged commit f2c137d into PyPSA:master May 13, 2025
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants