qpv-research-group / solcore5

A multi-scale, python-based library for the modelling of solar cells and semiconductor materials
https://www.solcore.solar/
Other
133 stars 77 forks source link

Remove the cross-compilation for MacOs - arm64? #269

Closed Abelarm closed 7 months ago

Abelarm commented 9 months ago

Reading this https://github.blog/changelog/2024-01-30-github-actions-introducing-the-new-m1-macos-runner-available-to-open-source/

looks like the github actions have now support in beta for machine with native arm64 based on the M1.

Should we test it and remove all the code that we made just for supporting the cross-compiling?

phoebe-p commented 8 months ago

Thanks for alerting me to this! I just went down a different rabbit hole when I started looking into this, because I realised the scheduled tests had been failing for ages on macOS because of a problem with installing S4. I have tried everything I can think of (on both the Intel Mac runners and the new M1 ones) but I cannot get S4 to compile successfully anymore, even though it works absolutely fine on my own M1 Mac. Since we are testing Solcore here and not the ability to compile S4, I have just removed it from the macOS tests for now. This shouldn't affect the build workflow though, so I will give that a try this week. One thing is that the M1 runners only have access to Python 3.10 and above, but I think that's ok.

phoebe-p commented 8 months ago

Ok, this seems to work (#270)! Will test the M1 wheel later. https://github.com/qpv-research-group/solcore5/actions/runs/8242553547

Abelarm commented 8 months ago

Happy to hear that at least it compiles!

Let's wait to see if it works🤞

Next week I can check the error of S4 which I remembered were working fine🤔

phoebe-p commented 8 months ago

It's odd. There seems to be at least four (!) different issues:

So to summarise, the M1 runners are stuck on not being able to find the Boost library, while the Intel runners are stuck on this flat namespace error. Just wrote this all out also as a summary for myself in case I come back to it, but unless you are interested in figuring it out, this is not an urgent problem -- the tests still run fine on Ubuntu, and in any case I am hoping to phase out the use of S4 for new users since I have introduced an interface with Inkstone which does the same thing but is Python-only and so much less of a pain to install (to be honest, I am tired of answering people's emails about installing S4....)

Abelarm commented 8 months ago

This a lot of information to digest, lucky there was the summary. I could try to look into the errors BUT if you think moving to a python only library would be easier in the long run, let's go for it.

I can help you with the latter as well, if you create some issues on refactoring from 'S4' to 'Inkstone' I can work on it. 🚀

Abelarm commented 8 months ago

Hi @phoebe-p,

good news here! 🚀

I managed to fix the build of S4 for macOS-arm64, you can see it in this PR https://github.com/phoebe-p/S4/pull/9

The fix consists manly in two part:

For now I fixed the pipelines by:

PR LINK https://github.com/qpv-research-group/solcore5/pull/272 If you accept it you'll need to accept first the PR I created for the S4 repo


I am still not able to build S4 on macOS-x86 my feeling is that we are not currently passing some parameters to the build of S4, because taking one command that is created with the Makefile from the build of S4 on MacOS-arm64 (correct) we got: clang++ -c -Wall -O3 -m64 -mcpu=apple-m1 -mtune=native -fPIC -Wall -I. -IS4 -IS4/RNP -IS4/kiss_fft -I/opt/homebrew/include/ -lboost_serialization -DHAVE_BLAS -DHAVE_LAPACK -DHAVE_FFTW3 -I/opt/homebrew/include/ -DHAVE_LIBCHOLMOD -I/opt/homebrew/include/suitesparse S4/rcwa.cpp -o build/S4k/rcwa.o

Instead the one from MacOS-x86 (incorrect):

clang++ -c -Wall -O3 -m64 -march=native -mtune=native -msse3 -msse2 -msse -fPIC -Wall -I. -IS4 -IS4/RNP -IS4/kiss_fft -DHAVE_BLAS -DHAVE_LAPACK -DHAVE_FFTW3 -DHAVE_LIBCHOLMOD -I/usr/local/include/suitesparse S4/rcwa.cpp -o build/S4k/rcwa.o

for me we are missing the -I/opt/homebrew/include/ -lboost_serialization part...

I tried to change the BOOST variables in the Makefile.mac_intel but still the parameters are not passed to build, I really don't know why 😢

phoebe-p commented 7 months ago

Thank you! No idea why the same approach for the Makefile does not seem to work on the Intel Mac, when it used to work perfectly fine. Have merged your PR on my S4 fork and will merge #272 here.