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

readme: add warning for "tables" dependency on Apple Silicon #77

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

AldoGl
Copy link
Contributor

@AldoGl AldoGl commented Oct 24, 2023

Proposed changes

I add a description on how to circumvent a known installation problem on Apple Silicon machines.

@AldoGl AldoGl force-pushed the readme_apple_silicon branch from 7278ef4 to 9cb07d0 Compare October 24, 2023 14:05
@codecov-commenter
Copy link

codecov-commenter commented Oct 24, 2023

Codecov Report

Merging #77 (9b7b8c1) into main (1904002) will not change coverage.
The diff coverage is n/a.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main      #77   +/-   ##
=======================================
  Coverage   89.22%   89.22%           
=======================================
  Files          42       42           
  Lines        1782     1782           
=======================================
  Hits         1590     1590           
  Misses        192      192           

This change was needed because of a nondeterminism in how the macos-12 (=macos-latest at the time of writing) runner image was instantiated by GitHub Actions.
In particular, for runner version 2.311.0, the operating system was nondeterministically chosen between 12.6.9 and 12.7.1, as it can be seen in these two GH runs:
- https://github.com/bancaditalia/black-it/actions/runs/7128997647/job/19412070642
- https://github.com/bancaditalia/black-it/actions/runs/7128997647/job/19412070868

This is a known issue in the community: see actions/runner-images#8642

Furthermore, at the time of writing, it is suspected that Python code that uses pytables is not stable over macos at version 12.7.1 (but this problem does not occur on v12.6.9!), as witnessed by this issue: PyTables/PyTables#1093

What happened in our CI workflow is that tests resulted in a flaky outcome, but not because of our tests, but due to the nondeterminism in how the macos runner was instantiated by GitHub Actions.

This was not our unique attempt. In fact, we first tried to improve the preliminary configuration steps for the testing environment, as follows:
- if: matrix.os == 'macos-latest'
      name: Install dependencies (macos-latest)
      run: |
        # for some reason this seems to be a necessary step for MacOS images, but not for Ubuntu and Windows
        brew install hdf5
        brew install c-blosc
        # https://stackoverflow.com/questions/73029883/could-not-find-hdf5-installation-for-pytables-on-m1-mac
        echo "HDF5_DIR=/usr/local/" >> $GITHUB_ENV
        echo "BLOSC_DIR=/usr/local/" >> $GITHUB_ENV

This attempt consisted in installing HDF5 and C-blosc explicitly in the preliminary steps on CI for MacOS images. By the way, this step was part of the installation instructions of PyTables v3.9.2: https://github.com/PyTables/PyTables/blob/v3.9.2/README.rst#installation , although it seems this step is not actually needed.
Moreover, as explained here: https://stackoverflow.com/questions/73029883/could-not-find-hdf5-installation-for-pytables-on-m1-mac, sometimes must be explicitly set the environment path variables where HDF5 ahd C-blosc are installed
For some reason this is not needed on Ubuntu and Windows runners.

However, despite our efforts in this direction, the issue with pytables persisted. From the logs of the CI: (https://github.com/bancaditalia/black-it/actions/runs/7121188681/job/19389914846?pr=78#step:5:62)

```
Fatal Python error: Illegal instruction

Current thread 0x000000011783c600 (most recent call first):
  File "/Users/runner/work/black-it/black-it/.tox/py310/lib/python3.10/site-packages/tables/node.py", line 246 in __init__
  File "/Users/runner/work/black-it/black-it/.tox/py310/lib/python3.10/site-packages/tables/leaf.py", line 264 in __init__
  File "/Users/runner/work/black-it/black-it/.tox/py310/lib/python3.10/site-packages/tables/carray.py", line 200 in __init__
  File "/Users/runner/work/black-it/black-it/.tox/py310/lib/python3.10/site-packages/tables/earray.py", line 143 in __init__
  File "/Users/runner/work/black-it/black-it/.tox/py310/lib/python3.10/site-packages/tables/file.py", line 1345 in create_earray
  File "/Users/runner/work/black-it/black-it/black_it/utils/json_pandas_checkpointing.py", line 211 in save_calibrator_state
  File "/Users/runner/work/black-it/black-it/tests/test_utils/test_pandas_json_checkpointing.py", line 58 in test_save_and_load_calibrator_state
  File "/Users/runner/work/black-it/black-it/.tox/py310/lib/python3.10/site-packages/_pytest/python.py", line 194 in pytest_pyfunc_call
  File "/Users/runner/work/black-it/black-it/.tox/py310/lib/python3.10/site-packages/pluggy/_callers.py", line 77 in _multicall
  File "/Users/runner/work/black-it/black-it/.tox/py310/lib/python3.10/site-packages/pluggy/_manager.py", line 115 in _hookexec
  File "/Users/runner/work/black-it/black-it/.tox/py310/lib/python3.10/site-packages/pluggy/_hooks.py", line 493 in __call__
  File "/Users/runner/work/black-it/black-it/.tox/py310/lib/python3.10/site-packages/_pytest/python.py", line 1792 in runtest
  File "/Users/runner/work/black-it/black-it/.tox/py310/lib/python3.10/site-packages/_pytest/runner.py", line 169 in pytest_runtest_call
  File "/Users/runner/work/black-it/black-it/.tox/py310/lib/python3.10/site-packages/pluggy/_callers.py", line 77 in _multicall
  File "/Users/runner/work/black-it/black-it/.tox/py310/lib/python3.10/site-packages/pluggy/_manager.py", line 115 in _hookexec
  File "/Users/runner/work/black-it/black-it/.tox/py310/lib/python3.10/site-packages/pluggy/_hooks.py", line 493 in __call__
  File "/Users/runner/work/black-it/black-it/.tox/py310/lib/python3.10/site-packages/_pytest/runner.py", line 262 in <lambda>
  File "/Users/runner/work/black-it/black-it/.tox/py310/lib/python3.10/site-packages/_pytest/runner.py", line 341 in from_call
  File "/Users/runner/work/black-it/black-it/.tox/py310/lib/python3.10/site-packages/_pytest/runner.py", line 261 in call_runtest_hook
  File "/Users/runner/work/black-it/black-it/.tox/py310/lib/python3.10/site-packages/_pytest/runner.py", line 222 in call_and_report
  File "/Users/runner/work/black-it/black-it/.tox/py310/lib/python3.10/site-packages/_pytest/runner.py", line 133 in runtestprotocol
  File "/Users/runner/work/black-it/black-it/.tox/py310/lib/python3.10/site-packages/_pytest/runner.py", line 114 in pytest_runtest_protocol
  File "/Users/runner/work/black-it/black-it/.tox/py310/lib/python3.10/site-packages/pluggy/_callers.py", line 77 in _multicall
  File "/Users/runner/work/black-it/black-it/.tox/py310/lib/python3.10/site-packages/pluggy/_manager.py", line 115 in _hookexec
  File "/Users/runner/work/black-it/black-it/.tox/py310/lib/python3.10/site-packages/pluggy/_hooks.py", line 493 in __call__
  File "/Users/runner/work/black-it/black-it/.tox/py310/lib/python3.10/site-packages/_pytest/main.py", line 350 in pytest_runtestloop
  File "/Users/runner/work/black-it/black-it/.tox/py310/lib/python3.10/site-packages/pluggy/_callers.py", line 77 in _multicall
  File "/Users/runner/work/black-it/black-it/.tox/py310/lib/python3.10/site-packages/pluggy/_manager.py", line 115 in _hookexec
  File "/Users/runner/work/black-it/black-it/.tox/py310/lib/python3.10/site-packages/pluggy/_hooks.py", line 493 in __call__
  File "/Users/runner/work/black-it/black-it/.tox/py310/lib/python3.10/site-packages/_pytest/main.py", line 325 in _main
  File "/Users/runner/work/black-it/black-it/.tox/py310/lib/python3.10/site-packages/_pytest/main.py", line 271 in wrap_session
  File "/Users/runner/work/black-it/black-it/.tox/py310/lib/python3.10/site-packages/_pytest/main.py", line 318 in pytest_cmdline_main
  File "/Users/runner/work/black-it/black-it/.tox/py310/lib/python3.10/site-packages/pluggy/_callers.py", line 77 in _multicall
  File "/Users/runner/work/black-it/black-it/.tox/py310/lib/python3.10/site-packages/pluggy/_manager.py", line 115 in _hookexec
  File "/Users/runner/work/black-it/black-it/.tox/py310/lib/python3.10/site-packages/pluggy/_hooks.py", line 493 in __call__
  File "/Users/runner/work/black-it/black-it/.tox/py310/lib/python3.10/site-packages/_pytest/config/__init__.py", line 169 in main
  File "/Users/runner/work/black-it/black-it/.tox/py310/lib/python3.10/site-packages/_pytest/config/__init__.py", line 192 in console_main
  File "/Users/runner/work/black-it/black-it/.tox/py310/bin/pytest", line 8 in <module>
```

Therefore, the current workaround we adopted was to downgrade the version of the macos runner to the previous version, 11.
@marcofavoritobi marcofavoritobi force-pushed the readme_apple_silicon branch 8 times, most recently from 0b65803 to f0c9400 Compare December 7, 2023 14:30
@marcofavoritobi marcofavoritobi force-pushed the readme_apple_silicon branch 2 times, most recently from 2489f3f to 9b7b8c1 Compare December 7, 2023 14:40
@marcofavoritobi marcofavoritobi force-pushed the readme_apple_silicon branch 7 times, most recently from 1445183 to 9276120 Compare December 7, 2023 16:00
@muxator muxator force-pushed the readme_apple_silicon branch from 9276120 to 339ff37 Compare February 23, 2024 19:44
@muxator
Copy link

muxator commented Feb 23, 2024

This PR worked around two issues:

  1. a nondeterminism in how the macos-12 images were instantiated by Github Actions.
  2. an incompatibility between PyTables 3.9.2 (but we use 3.8.0) and macos 12.7.1, which is being investigated at tables 3.9.2 problem on macos CI  PyTables/PyTables#1093, and which is still open

The first one seems to be fixed. See actions/runner-images#8642 (comment):

The problem have been resolved now, sorry for inconvenience!

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.

4 participants