Error Building Wheel on MacOS

[fcotizelati]

Hello,

I am writing to report an issue while attempting to install the package via pip. Here are the details.

Operating System: MacOS 14.1 Python Version: 3.11.6 Pip Version: pip 23.3.1

During the installation process, an error occurred while building the wheel. Here is the excerpt from the command line output:

Collecting riptide-ffa Using cached riptide-ffa-0.2.4.tar.gz (77 kB) Installing build dependencies: started Installing build dependencies: finished with status 'done' Getting requirements to build wheel: started Getting requirements to build wheel: finished with status 'done' Preparing metadata (pyproject.toml): started Preparing metadata (pyproject.toml): finished with status 'done' Requirement already satisfied: astropy in /usr/local/lib/python3.11/site-packages (from riptide-ffa) (5.3) Requirement already satisfied: pandas in /usr/local/lib/python3.11/site-packages (from riptide-ffa) (2.1.2) Requirement already satisfied: schema in /usr/local/lib/python3.11/site-packages (from riptide-ffa) (0.7.5) Requirement already satisfied: pyyaml in /usr/local/lib/python3.11/site-packages (from riptide-ffa) (6.0.1) Requirement already satisfied: threadpoolctl in /usr/local/lib/python3.11/site-packages (from riptide-ffa) (3.1.0) Collecting pytest (from riptide-ffa) Using cached pytest-7.4.3-py3-none-any.whl.metadata (7.9 kB) Collecting pytest-cov (from riptide-ffa) Using cached pytest_cov-4.1.0-py3-none-any.whl.metadata (26 kB) Requirement already satisfied: matplotlib in /usr/local/lib/python3.11/site-packages (from riptide-ffa) (3.8.2) Requirement already satisfied: numpy in /usr/local/lib/python3.11/site-packages (from riptide-ffa) (1.26.2) Requirement already satisfied: pyerfa>=2.0 in /usr/local/lib/python3.11/site-packages (from astropy->riptide-ffa) (2.0.0.3) Requirement already satisfied: packaging>=19.0 in /usr/local/lib/python3.11/site-packages (from astropy->riptide-ffa) (23.2) Requirement already satisfied: contourpy>=1.0.1 in /usr/local/lib/python3.11/site-packages (from matplotlib->riptide-ffa) (1.1.1) Requirement already satisfied: cycler>=0.10 in /usr/local/lib/python3.11/site-packages (from matplotlib->riptide-ffa) (0.12.1) Requirement already satisfied: fonttools>=4.22.0 in /usr/local/lib/python3.11/site-packages (from matplotlib->riptide-ffa) (4.43.1) Requirement already satisfied: kiwisolver>=1.3.1 in /usr/local/lib/python3.11/site-packages (from matplotlib->riptide-ffa) (1.4.5) Requirement already satisfied: pillow>=8 in /usr/local/lib/python3.11/site-packages (from matplotlib->riptide-ffa) (10.1.0) Requirement already satisfied: pyparsing>=2.3.1 in /usr/local/lib/python3.11/site-packages (from matplotlib->riptide-ffa) (3.1.1) Requirement already satisfied: python-dateutil>=2.7 in /usr/local/lib/python3.11/site-packages (from matplotlib->riptide-ffa) (2.8.2) Requirement already satisfied: pytz>=2020.1 in /usr/local/lib/python3.11/site-packages (from pandas->riptide-ffa) (2023.3) Requirement already satisfied: tzdata>=2022.1 in /usr/local/lib/python3.11/site-packages (from pandas->riptide-ffa) (2023.3) Collecting iniconfig (from pytest->riptide-ffa) Using cached iniconfig-2.0.0-py3-none-any.whl (5.9 kB) Collecting pluggy<2.0,>=0.12 (from pytest->riptide-ffa) Using cached pluggy-1.3.0-py3-none-any.whl.metadata (4.3 kB) Requirement already satisfied: coverage>=5.2.1 in /usr/local/lib/python3.11/site-packages (from coverage[toml]>=5.2.1->pytest-cov->riptide-ffa) (7.2.7) Requirement already satisfied: contextlib2>=0.5.5 in /usr/local/lib/python3.11/site-packages (from schema->riptide-ffa) (21.6.0) Requirement already satisfied: six>=1.5 in /usr/local/lib/python3.11/site-packages (from python-dateutil>=2.7->matplotlib->riptide-ffa) (1.16.0) Using cached pytest-7.4.3-py3-none-any.whl (325 kB) Using cached pytest_cov-4.1.0-py3-none-any.whl (21 kB) Using cached pluggy-1.3.0-py3-none-any.whl (18 kB) Building wheels for collected packages: riptide-ffa Building wheel for riptide-ffa (pyproject.toml): started Building wheel for riptide-ffa (pyproject.toml): finished with status 'error' Failed to build riptide-ffa

Thank you for your help, Francesco

[v-morello]

Hi Francesco,

Just to be sure: did you install via pip install riptide-ffa, or did you clone the repo and then installed in editable mode (which is what make install does) ?

Whichever option you chose, I would need you to run the associated pip command you used again, but with maximum verbosity (add the option -vvv to the command), and post what happens between those steps:

Building wheel for riptide-ffa (pyproject.toml): started Building wheel for riptide-ffa (pyproject.toml): finished with status 'error'

As a general comment, I never committed to fully supporting MacOS because back when I was actively developing riptide, the CI pipelines' support for testing on MacOS was shaky as far as I remember. But in practice, "riptide sort of works on MacOS 95% of the time" is my official line. If I can find time to do better than that, I'll do it, but no promises. In any case, I'll help you with fixiing your specific issue here.

[fcotizelati]

Hi Vincent,

Thank you for your guidance on debugging this issue.

I used the pip install riptide-ffa method for installation. I ran the command again adding the -vvv option but it seems that this does not capture additional information about what happens between those two steps. I've attached the output of the command "pip install riptide-ffa -vvv > output.txt"

Thank you, Francesco

output.txt

[v-morello]

Indeed there is not much more information, but after some googling, I found that the command to run to get the answer we need is: pip install riptide-ffa --log piplog.txt

Please attach piplog.txt here and hopefully we can get to the bottom of this without too much effort. Edit: I created a fresh Python 3.11 + pip 23.3.1 environment on my Linux machine, installed riptide using pip and everything works. It's not a Python version problem we seem to be facing here, but a Mac-specific issue.

[fcotizelati]

Thank you for your checks, it seems indeed a Mac-specific issue.

I'm attaching the generated piplog.txt file.

piplog.txt

[v-morello]

There we have it, near the end of the file, it's a C++ compilation error:

riptide/cpp/python_bindings.cpp:37:63: error: non-constant-expression cannot be narrowed from type 'size_t' (aka 'unsigned long') to 'ssize_t' (aka 'long') in initializer list [-Wc++11-narrowing]

auto arr_output = py::array_t<float, py::array::c_style>({size});
                                                         ^~~~
 riptide/cpp/python_bindings.cpp:37:63: note: insert an explicit cast to silence this issue

There are multiple instances of this same error. I'm surprised that the constructor of array_t wants its size/shape argument as a signed int rather than unsigned, but whatever. It is an easy fix. You can try to fix it yourself locally on your machine, but for that you'll have to install with the second method (clone the repo, make install, and after any edit of the C++ code, make install again to rebuild the C++ extensions). If you're not in a hurry, I can do it myself probably later this week. In any case, I will have to make a new release on PyPI for everyone to enjoy the changes.

[fcotizelati]

Thank you for pointing out the compilation error. I can certainly attempt to fix it locally by following the second installation method, and I wait for the updated release on PyPI. Thanks again for your help.

[v-morello]

Please let me know of your progress :smile:

[v-morello]

@fcotizelati Have you managed to fix this locally ?

[fcotizelati]

Sorry for the delay in responding. I haven't reported any updates because in these days I made some attempts by trying to modify a few lines of the python_bindings.cpp file and recompiling, but I always get the same error that is reported in the piplog.txt which I previously attached. I don't know if the solution is simpler than what I think. By any chance, have you had the opportunity and time to work on fixing the issue?

[v-morello]

Hi Francesco, I had a look into the problem for about an hour this morning, and I managed to reproduce the issue by installing a recent version of clang on my Linux machine. In the end it's not really a MacOS issue, it is simply that the C++17 standard is more strict about narrowing conversions. I also found out during that hacking session that recent gcc versions report those same clang errors as warnings, so it's only a matter of time before the C++ extensions build of riptide starts breaking everywhere.

Plan: I will try fixing this later today/tomorrow, if I find a solution I will push it to a separate branch and you can try that for yourself.

[fcotizelati]

Hi Vincent, thanks a lot for the update and your efforts in diagnosing the issue with Riptide's C++ extensions build. Once you have a potential fix and push it to a separate branch, I'll be happy to test it on my end!

[v-morello]

Good news: the narrowing conversion error was easy to fix.

Surprise along the way: fixing that revealed a much worse issue: when compiling the C++ extensions with clang, one unit test systematically raises a segmentation fault. I spent hours investigating this, and it turns out that mindlessly using the flag -ffast-math on some compilers is a terrible idea (2018 me was more carefree than 2023 me). Here it causes surprising inconsistencies when calculating the size of the output periodogram, and the size of the buffers to store some intermediate data products (e.g. the input data downsampled by a real-valued factor). The code then unexpectedly attempts to write past the end of these arrays.

In the end, I found the subset of optimisation flags enabled by -ffast-math that can be safely turned on, and luckily without losing any speed compared to just enabling -ffast-math.

Anyway, I'm almost there. I definitely need to make a new release soon :smile:

[v-morello]

@fcotizelati Fixes are now in the dev branch. Please pull and checkout the dev branch, then pip uninstall riptide-ffa and and re-install in editable mode.

[fcotizelati]

Thank you @v-morello for all your work on resolving the issues. I've just pulled the latest updates from the dev branch and reinstalled riptide-ffa successfully in editable mode. I've also run the test suite in Python without any problems :)

[v-morello]

Great, I will close this once I've pushed a new patch release with the fixes on PyPI.

[scottransom]

Interesting about -ffast-math. I've had that on as a default in PRESTO for years. I don't think it is causing any issues that I've seen. But this is good to know!

[v-morello]

Hi @scottransom , glad to see you around here :smile:

Here's an interesting read I dug up on the subject yesterday: https://simonbyrne.github.io/notes/fastmath/

I think -ffast-math is absolutely fine in most data processing, but in riptide I am playing with fire by pre-allocating some output arrays to the exact required size, and that calculation better be correct, if you've seen the 2015 movie "The Martian":

/*
Number of samples in a time series after it has been downsampled by a real-valued factor f
*/
size_t downsampled_size(size_t num_samples, double f)
    {
    return floor(num_samples / f);
    }

Here it was specifcally the -freciprocal-math flag that was causing trouble when num_samples / f was extremely close to an integer, and the result could be one smaller than expected, with obvious consequences.

[v-morello]

Both issues fixed in v0.2.5, which is now live on PyPI. Closing this.