search

NNPDF / pineappl

PineAPPL is not an extension of APPLgrid
https://nnpdf.github.io/pineappl/
GNU General Public License v3.0
12 stars 3 forks source link

Fix MacOS Python wheel generation #262

Closed cschwan closed 4 months ago

alecandido commented 4 months ago

@cschwan did you just give up on fixing the Python range? Or is still an attempt to test it?

In case it's the latter: what are you testing?

alecandido commented 4 months ago

Btw, since we're touching the version range, consider that also py3.7 is EOL by last June.

https://devguide.python.org/versions/

The current supported range is 3.8-3.12, but I saw many popular packages (including NumPy, that is relevant for us) dropping support for py3.8 already (it will be EOL by October, together with the release of py3.13). https://pypi.org/project/numpy/#files

cschwan commented 4 months ago

I don't think the Python range has any influence on the subsequent maturin action and, that's what I want to test, possible a harmful influence. In any case I'm just using what maturin generate-ci github produces and if that doesn't work it'll warrant and Issue on maturin's Github page.

alecandido commented 4 months ago

I don't think the Python range has any influence on the subsequent maturin action and, that's what I want to test, possible a harmful influence.

Ok, so your guess is that the interpreter found by maturin on MacOS are all already present on the runner, isn't it?

For manylinux, this depends on the container, so we already know to be unrelated to the host. But for MacOS it would be a bit surprising (setup-python should install at least user-wide, but I assumed system-wide - so, I'd be puzzled by the --find-interpreter discovery mechanism).

alecandido commented 4 months ago

Ah @cschwan, while scanning the workflow, I noticed: https://github.com/NNPDF/pineappl/blob/00140d5dad88ddf6b5ed2e3cf6a429668334ef6b/.github/workflows/release.yml#L137 which is different from YAML's boolean:

    sccache: true

I would not touch in this PR, or not before having fixed the Python MacOS issue (to avoid noise).

cschwan commented 4 months ago

Despite what I said it seems to make a difference; before commit 00140d5dad88ddf6b5ed2e3cf6a429668334ef6b maturin --find-interpreter finds:

Found PyPy 3.7 at /Users/runner/hostedtoolcache/PyPy/3.7.13/x64/bin/python3.7,
      PyPy 3.8 at /Users/runner/hostedtoolcache/PyPy/3.8.16/x64/bin/python3.8,
      PyPy 3.9 at /Users/runner/hostedtoolcache/PyPy/3.9.18/x64/bin/python3.9,
      PyPy 3.10 at /Users/runner/hostedtoolcache/PyPy/3.10.13/x64/bin/python3.10,
      CPython 3.11 at /Library/Frameworks/Python.framework/Versions/3.11/bin/python3.11,
      CPython 3.12 at /usr/local/bin/python3.12,
      PyPy 3.7 at /Users/runner/hostedtoolcache/PyPy/3.7.13/x64/bin/pypy3.7,
      PyPy 3.8 at /Users/runner/hostedtoolcache/PyPy/3.8.16/x64/bin/pypy3.8,
      PyPy 3.9 at /Users/runner/hostedtoolcache/PyPy/3.9.18/x64/bin/pypy3.9,
      PyPy 3.10 at /Users/runner/hostedtoolcache/PyPy/3.10.13/x64/bin/pypy3.10

after the commit it finds:

Found CPython 3.10 at /Users/runner/hostedtoolcache/Python/3.10.13/x64/bin/python3.10,
      CPython 3.11 at /usr/local/bin/python3.11,
      CPython 3.12 at /usr/local/bin/python3.12
alecandido commented 4 months ago

That's incredibly weird, and the most incredible part is CPython 3.10: how come that is found only when installed in isolation?

alecandido commented 4 months ago

I want to test whether PyPy is masking CPython in discovery

cschwan commented 4 months ago

The last version set in python-version is the default version, so maybe something goes wrong when maturin is run with PyPy.

cschwan commented 4 months ago

This looks much better now:

Found CPython 3.7m at /Users/runner/hostedtoolcache/Python/3.7.17/x64/bin/python3.7,
      CPython 3.8 at /Users/runner/hostedtoolcache/Python/3.8.18/x64/bin/python3.8,
      CPython 3.9 at /Users/runner/hostedtoolcache/Python/3.9.18/x64/bin/python3.9,
      CPython 3.10 at /Users/runner/hostedtoolcache/Python/3.10.13/x64/bin/python3.10,
      CPython 3.11 at /Library/Frameworks/Python.framework/Versions/3.11/bin/python3.11,
      CPython 3.12 at /Library/Frameworks/Python.framework/Versions/3.12/bin/python3.12
alecandido commented 4 months ago

This looks much better now:

Found CPython 3.7m at /Users/runner/hostedtoolcache/Python/3.7.17/x64/bin/python3.7,
      CPython 3.8 at /Users/runner/hostedtoolcache/Python/3.8.18/x64/bin/python3.8,
      CPython 3.9 at /Users/runner/hostedtoolcache/Python/3.9.18/x64/bin/python3.9,
      CPython 3.10 at /Users/runner/hostedtoolcache/Python/3.10.13/x64/bin/python3.10,
      CPython 3.11 at /Library/Frameworks/Python.framework/Versions/3.11/bin/python3.11,
      CPython 3.12 at /Library/Frameworks/Python.framework/Versions/3.12/bin/python3.12

Where did you find this info in the logs?

cschwan commented 4 months ago

This looks much better now:

Found CPython 3.7m at /Users/runner/hostedtoolcache/Python/3.7.17/x64/bin/python3.7,
      CPython 3.8 at /Users/runner/hostedtoolcache/Python/3.8.18/x64/bin/python3.8,
      CPython 3.9 at /Users/runner/hostedtoolcache/Python/3.9.18/x64/bin/python3.9,
      CPython 3.10 at /Users/runner/hostedtoolcache/Python/3.10.13/x64/bin/python3.10,
      CPython 3.11 at /Library/Frameworks/Python.framework/Versions/3.11/bin/python3.11,
      CPython 3.12 at /Library/Frameworks/Python.framework/Versions/3.12/bin/python3.12

Where did you find this info in the logs?

Here: https://github.com/NNPDF/pineappl/actions/runs/8016527785/job/21898622667#step:4:242. I added some newlines to improve the readibility.

alecandido commented 4 months ago

Concerning maturin: I investigated the source, and there are a lot of non-trivial heuristics that takes into account the platform, the Python kind, and so on.

However, the essential mechanism is simple:

So, it truly depends on $PATH availability, plus complications to determine the Python specs, that could affect the validity of the executable - but I expect them not to be our issue. However, if on Linux is working simultaneously for CPython and PyPy, the problem could be inside the complications related to the MacOS target.

alecandido commented 4 months ago

So, does it truly depend on the installation order?

🐍 Found CPython 3.7m at /Users/runner/hostedtoolcache/Python/3.7.17/x64/bin/python3.7,
         CPython 3.8 at /Users/runner/hostedtoolcache/Python/3.8.18/x64/bin/python3.8,
         CPython 3.9 at /Users/runner/hostedtoolcache/Python/3.9.18/x64/bin/python3.9,
         CPython 3.10 at /Users/runner/hostedtoolcache/Python/3.10.13/x64/bin/python3.10,
         CPython 3.11 at /Library/Frameworks/Python.framework/Versions/3.11/bin/python3.11,
         CPython 3.12 at /Library/Frameworks/Python.framework/Versions/3.12/bin/python3.12,
         PyPy 3.7 at /Users/runner/hostedtoolcache/PyPy/3.7.13/x64/bin/pypy3.7,
         PyPy 3.8 at /Users/runner/hostedtoolcache/PyPy/3.8.16/x64/bin/pypy3.8,
         PyPy 3.9 at /Users/runner/hostedtoolcache/PyPy/3.9.18/x64/bin/pypy3.9,
         PyPy 3.10 at /Users/runner/hostedtoolcache/PyPy/3.10.13/x64/bin/pypy3.10
cschwan commented 4 months ago

It can't (?) depend on the installation order, but I believe the default Python version (with which maturin is started) does make a difference.

cschwan commented 4 months ago

@alecandido with commit f00839ecbf8516da69572d9ecfba7f8dafdb944f I've switched the wheel generation from universal wheels to two separate wheels, one for each of the two (aarch and x86) targets. They're potentially generated more quickly and the wheels themselves are now smaller. Do you think this is OK or should we go back to universal wheels?

alecandido commented 4 months ago

@alecandido with commit f00839e I've switched the wheel generation from universal wheels to two separate wheels, one for each of the two (aarch and x86) targets. They're potentially generated more quickly and the wheels themselves are now smaller. Do you think this is OK or should we go back to universal wheels?

That's should be perfectly fine. It's weird that is faster than generating the universal one (I wonder what's the purpose of it at this point, unless you have GB of shared Python code and data...), but I expected to be more or less the same anyhow (it should compile twice, no way out: they are two different ISAs).

alecandido commented 4 months ago

It can't (?) depend on the installation order, but I believe the default Python version (with which maturin is started) does make a difference.

It could be, but I'm not sure about that. We could test moving a single PyPy version at the bottom, but if now it's working is probably not worth.

cschwan commented 4 months ago

It's weird that is faster than generating the universal one

What I meant but didn't say is that the two wheels can be run in parallel and thus are usually faster, unless of course no Mac runner is found (like yesterday).

alecandido commented 4 months ago

unless of course no Mac runner is found (like yesterday).

Unfortunately, that's usually the case. There are many limitations on Mac runners. But we don't care too much about CI performances. As long as they are reasonable that's enough.