giotto-ai / giotto-ph

High performance implementation of Vietoris-Rips persistence.
https://giotto-ai.github.io/giotto-ph/
Other
43 stars 12 forks source link

Failed dev installation on MacOS #2

Closed giotto-learn closed 3 years ago

giotto-learn commented 3 years ago

Brief description

The dev installation failed at the C++ compilation step.

Steps to reproduce

After cloning the repo and moving into the root folder of the repo, I launched

pip install -e .

The installation failed. I tried on both branches (main and c++_backend)

Expected behaviour

The dev installation should not fail

Logs

logs.txt

Possible solution

My suggestion is to build the wheels in the CI and publish then as artefacts. The following code can be added directly, after the testing phase (currently done with pytest) in the CI:

    - name: Build and install wheels
      run: |
        set -e
        python -m pip install wheel
        python setup.py bdist_wheel
        python -m pip install dist/*.whl
    - name: Upload artifacts
      uses: actions/upload-artifact@v2
      with:
        name: pip_wheel_${{ matrix.python-version }}
        path: dist

One can then simply download the wheels and install them directly with pip. This should avoid compilers problems.

MonkeyBreaker commented 3 years ago

Hi,

From the discussion on Slack it seems that the reason of it didn't compile on Mac was because the compiler used didn't support C++14 standard. I just add a check in #1 in order to detect at compilation time if the compiler can process our library. Hope now when this happen again, the installation tells you that your compiler does not support the compilation of giotto-ph.

Sorry for the issue, Julián

ulupo commented 3 years ago

This relates to another issue: currently, for the developer install, we just send to the giotto-tda instructions. This is confusing to the reader, and misleading because giotto-ph does not need Boost.

MonkeyBreaker commented 3 years ago

I think that we should close this issue for the release, i could confuse users at it is just in my opinion that the compiler used does not support C++14 features.