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

set CC env variable #266

Merged
merged 2 commits into from
Jan 16, 2025
Merged

set CC env variable #266

merged 2 commits into from
Jan 16, 2025

Conversation

bknight1
Copy link
Member

@bknight1 bknight1 commented Nov 8, 2024

Fixes issues with finding mpi

Fixes issues with finding mpi
@gthyagi
Copy link
Contributor

gthyagi commented Jan 15, 2025

@bknight1 please change main to development here too. Thanks.
@julesghub please merge this PR as well. Thanks.

@bknight1 bknight1 changed the base branch from main to development January 15, 2025 04:02
@julesghub
Copy link
Member

I'm unclear if this approach is the best.
As I read this we are only overwriting CC so cython can compile the extensions correctly, is that correct?
This doesn't happen on my lappy. Any specific reason why it happens on yours?

@bknight1
Copy link
Member Author

@julesghub This was to fix a changein mpi4py>=4 where we can't get the CC env variable from mpi4py any more. UW won't compile if the CC env isn't set as it can't find the mpi install.

@bknight1
Copy link
Member Author

Could add a flag to only set it if it's not found?

@julesghub
Copy link
Member

interesting, I'm running mpi4py==4.0.1 and have no issues.
I wonder if this a mac thing - I can test more tomorrow.

@bknight1
Copy link
Member Author

Is your CC variable already set in terminal?

@julesghub
Copy link
Member

Nope.

@bknight1
Copy link
Member Author

Okay, fair enough. Thyagi and I discussed in detail on #242

@julesghub
Copy link
Member

julesghub commented Jan 15, 2025

mpi4py/mpi4py#532
Relevant to the discussion. Why mpi4py no longer needs to be constraint to a single implementation of MPI.

@julesghub
Copy link
Member

julesghub commented Jan 16, 2025

ok, I gave it a go and I think this is the issue.
I installed mpi to system directories. But on mac people often install mpi with homebrew, potentially not in a system dir.
Hence cython can't find the mpi implementation in sys dirs and dies compiling uw3.

@bknight1 and @gthyagi are using homebrew?

@bknight1
Copy link
Member Author

Yeah, I used homebrew to install ompi

@julesghub
Copy link
Member

Yeah, I used homebrew to install ompi

What's your path?
Something not in system standard lib or inc dirs

@bknight1
Copy link
Member Author

bknight1 commented Jan 16, 2025

It's the homebrew dir:/opt/homebrew/bin/mpicc

which is included in my $PATH env variable, so should be picked up?

@julesghub
Copy link
Member

yeah the mpicc will be picked up but not the mpi.h, which was the main issue I believe.
So I think this PR is good to go - will accept it after lunch.

@gthyagi
Copy link
Contributor

gthyagi commented Jan 16, 2025

I am using manual install /Users/tgol0006/manual_install_pkg/openmpi-4.1.6/bin/mpicc. Even then I encounter that error in Mac.

@julesghub
Copy link
Member

@gthyagi you need to use mpicc, not mpirun.

Hopefully this is a fix for effiectively getting mpicc.
Need to test using conda next.
Copy link
Member Author

@bknight1 bknight1 left a comment

Choose a reason for hiding this comment

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

Looks good!

@julesghub
Copy link
Member

I add a rework to effectively get what petsc was compiled with, not directly get mpicc. Show work all the same.
This approach is slightly safer for those funny hpc issues where we have had different mpi implementations for UW and PETSC (and hdf5). Will test using conda soon. Could @gthyagi and @bknight1 please test and report back.
Thanks!

@gthyagi
Copy link
Contributor

gthyagi commented Jan 16, 2025

I am now able to install the code without any MPI issues using this fix. Please merge it.

@julesghub julesghub self-requested a review January 16, 2025 12:52
@julesghub julesghub merged commit cdd5fb0 into development Jan 16, 2025
1 check passed
@julesghub julesghub deleted the bknight1-mpi_fix branch January 16, 2025 12:53
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.

3 participants