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

123 mac m1 installation #164

Merged
merged 10 commits into from
Oct 16, 2023
Merged

123 mac m1 installation #164

merged 10 commits into from
Oct 16, 2023

Conversation

sy-cui
Copy link
Collaborator

@sy-cui sy-cui commented Oct 2, 2023

Fix #123

The source of dependencies PyFFTW and pystencils are set to two temporary git repositories.
PyFFTW: https://github.com/tp5uiuc/pyFFTW/tree/macos_fix
pystencils: https://github.com/sy-cui/pystencils/tree/macos_fix

The modifed PyFFTW looks up fftw library in /opt/homebrew/lib on macos platforms.

In pystencils, the shared object (.so) file is not dynamically linked to OpenMP properly on Apple M1. Restricting the -fopenmp compiler option to the linking stage alone seems to resolve this issue. For our purpose, it is also plausible to compile in one step instead of two

clang++ -shared -o object.so object.cpp -fopenmp -undefined dynamic_lookup [other options]

but we don't do it for now.

A new environment.yml file is provided to integrate poetry into conda environment.
(curtesy of https://stackoverflow.com/questions/70851048/does-it-make-sense-to-use-conda-poetry)

Poetry version is restricted to below 1.6 to avoid a known issue with version control.

Edit:
The changes regarding -fopenmp on pystencils is reverted as multithreading isn't enabled if the flag isn't passed. Instead the clang compiler flag -Xclang is removed as it prevents -fopenmp from being recognized as a regular command line option.

@sy-cui sy-cui added prio:high High priority dependencies Pull requests that update a dependency file labels Oct 2, 2023
@sy-cui sy-cui self-assigned this Oct 2, 2023
pyproject.toml Outdated Show resolved Hide resolved
Copy link
Contributor

@bhosale2 bhosale2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

pyproject.toml Show resolved Hide resolved
@bhosale2
Copy link
Contributor

@armantekinalp please take a look again and merge accordingly.

@armantekinalp
Copy link
Collaborator

I think this is not working yet. Songyuan was planning to test on different machines. @sy-cui can you confirm?

@sy-cui
Copy link
Collaborator Author

sy-cui commented Oct 11, 2023

After the most recent push everything should be fine now. I have tested on Bridges and Expanse and they install without issue. We could (and should) let people in the lab try installing now because I made the dependency sources platform dependent, and we might not catch some issues with just clusters.

@armantekinalp
Copy link
Collaborator

Okay let me try on my system as well, then we can proceed

@bhosale2 bhosale2 self-requested a review October 11, 2023 18:35
@bhosale2
Copy link
Contributor

bhosale2 commented Oct 11, 2023

@sy-cui and @armantekinalp current installation instructions on this branch didnt work on my machine (pyfftw issue I think):

Screenshot 2023-10-11 at 3 03 14 PM

@bhosale2
Copy link
Contributor

bhosale2 commented Oct 11, 2023

ok I just realized the issue. The install instructions don't tell to install fftw via brew first. @sy-cui lets add it somewhere then? or what if we integrate it in conda env?

Also we need instructions to install llvm and link it properly. @sy-cui please discuss with @armantekinalp and figure out a solution accordingly.

basically current instructions dont work off the bat, some more details are needed

@sy-cui
Copy link
Collaborator Author

sy-cui commented Oct 16, 2023

@armantekinalp @bhosale2 can we merge this?

Copy link
Collaborator

@armantekinalp armantekinalp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@bhosale2 bhosale2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bhosale2 bhosale2 merged commit 177b4ff into main Oct 16, 2023
1 check passed
@bhosale2 bhosale2 deleted the 123_mac_m1_installation branch October 16, 2023 22:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file prio:high High priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Installing SOPHT for M1 Macs
3 participants