Matrix testing on different python version still always uses python 3.12

[pmrv]

The unit tests seem to be run always with python 3.12, even if the unit test workflow says it's 3.9/3.10/whatever.
By accident I used a 3.12 syntax feature in this PR, which should (and does locally) make older pythons crash, but the unit test matrix runs fine. So I stuck import sys; print(sys.version) into the tests and it does report here that it runs 3.12 even though it's supposed to be the 3.9 test.

[pmrv]

I will revert the commits that added the print because I'd like the merge the corresponding PR soon, but I hope the link to the CI results stay alive. If not it's easy enough to recreate.

[liamhuber]

Yikes, good catch. I immediately assumed it was a simple error on my part in failing to pass an input variable through the call chain, but in fact the code chain looks fine:

• Is input to pyiron/actions/.github/workflows/push-pull.yml
• Used in the matrix for the unit-tests step
• Passed to pyiron/actions/unit-tests
• Passed to pyiron/actions/cached-miniforge
• Passed to conda-incubator/setup-miniconda

Then I realized I could just look at the logs to confirm this, and it's the same thing: the 3.9 flag is successfully getting passed all the way through to conda-incubator/setup-miniconda:

Indeed, the conda setup correctly shows 3.9 being requested:

And once we get the whole thing installed and run conda list for the logs, we still see the correct version of python:

I then thought maybe we mess up the system variables? But the pyiron config business sets CONDA: /home/runner/miniconda3, which is exactly correct.

After that we just coverage run. The only remaining possibility I see for an issue with the workflow is that this command is somehow invoking the wrong version of python, but still has all the right packages? This is weird though as the env reports look fine here too:

I will revert the commits that added the print because I'd like the merge the corresponding PR soon, but I hope the link to the CI results stay alive. If not it's easy enough to recreate.

Is there any possibility that somehow the reporting itself is the problem? I see you force-pushed in the linked PR so I can't see the source of the report. It seems extremely unlikely to me that the report is wrong, but given that the logs consistently show the right version throughout, I'm just very lost.

[jan-janssen]

It is very strange you have python =3.9.19 and python_abi =3.12 in the same environment, it should be python_abi =3.9. This seems to be related to the mamba env update command as I do not see the same issue on the repositories which directly set the environment-file in the conda-incubator/setup-miniconda action.

[liamhuber]

Very nice observation. So coverage run is really invoking the wrong executable.

Given that the python version kwarg is getting passed all the way to the conda incubator, it's a matter of seeing what other input we're passing that's creating a conflict (or, much less likely, there is a bug in the incubator action)

[liamhuber]

Also as far as I recall, we are writing our env file to the default location for the incubator setup, so even though it's not being set explicitly it should be getting passed in just fine. I'll double check that when I get to this though.

[liamhuber]

The problem appears a mixture of mamba update and having python versions explicitly in the env file. As you suggest, Jan, passing the env file directly to setup-miniconda solves this, but then we sacrifice caching.

The mamba update (and indeed the rest of the caching procedure) is exactly from the conda-incubator docs on caching, which is most certainly what I followed when building this action. The interplay between the action argument for python versions and the env file used in the update specifying python versions is just a really unfortunate edge case that they aren't aware of/don't bother documenting.

I'm working on a PR (#124) which solves this by decomposing the caching into two steps so we can cache and use the file directly at invocation of setup-miniconda. I'm quite confident it will work, but I still need to squash bugs in getting all the pathing working. I'll try to get to it tonight, but this might drag into tomorrow despite my promise to get a new release out by the end of the day.

[pmrv]

Thanks! Like I said it's a not a time critical issue, but good that you figured it out so quickly.

[jan-janssen]

The problem appears a mixture of mamba update and having python versions explicitly in the env file. As you suggest, Jan, passing the env file directly to setup-miniconda solves this, but then we sacrifice caching.

At least in my tests on the packages without pyiron/actions caching is no longer faster. Downloading the cache from Github takes longer than downloading the packages from anaconda.

[liamhuber]

The problem appears a mixture of mamba update and having python versions explicitly in the env file. As you suggest, Jan, passing the env file directly to setup-miniconda solves this, but then we sacrifice caching.

At least in my tests on the packages without pyiron/actions caching is no longer faster. Downloading the cache from Github takes longer than downloading the packages from anaconda.

My recent experience is that for very simple envs (the pyiron_snippets unit test env), loading the cache is indeed slightly slower than doing everything from scratch (35s vs 30s), but for a "complex" environment (pyiron_base + dask, neither with version pins) uncaching is significantly faster (27s vs 1m8s). Qualitatively, the gains on more complex setups seem worth risking the minor hit to performance for things that are already fast. Further, although my tests were N=1 and not scientific at all, un-caching seems pretty consistent -- so if you look at pyiron_atomistics where the mamba setup step is taking ~1.5m, we might be able to see some real gains.

[liamhuber]

The speedup between writing and reading the cache is significant for pyiron_workflow: Ubuntu about 1.5m -> 0.75m, windows from 5m -> 2m. OSX was waiting too long in the queue so I don't know the speedup.