Skip to content

Add MPIPreferences subpackage to select binary#541

Merged
simonbyrne merged 54 commits intomasterfrom
sb/mpipref
Apr 13, 2022
Merged

Add MPIPreferences subpackage to select binary#541
simonbyrne merged 54 commits intomasterfrom
sb/mpipref

Conversation

@simonbyrne
Copy link
Member

@simonbyrne simonbyrne commented Feb 15, 2022

Based on #528 and #529.

@simonbyrne
Copy link
Member Author

simonbyrne commented Feb 15, 2022

Note that the spurious tests are passing due to JuliaLang/Pkg.jl#2920

@simonbyrne
Copy link
Member Author

simonbyrne commented Feb 16, 2022

@vchuravy @staticfloat: until JuliaLang/Pkg.jl#2920 makes it into a new release, is there a way to work around the problem for testing purposes?

@simonbyrne simonbyrne closed this Feb 18, 2022
@simonbyrne simonbyrne reopened this Feb 18, 2022
@simonbyrne simonbyrne marked this pull request as ready for review February 21, 2022 06:08
giordano
giordano reviewed Feb 21, 2022
View reviewed changes
Copy link
Member

@giordano giordano left a comment

Choose a reason for hiding this comment

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

I think I missed how the constants have been generated. It'd be good to have a reproducible way to update them, if needed.

Also, the documentation should also be updated, right?

lib/MPIPreferences/src/MPIPreferences.jl
if (m = match(r"CRAY MPICH version (\d+.\d+.\d+)", version_string)) !== nothing
version = VersionNumber(m.captures[1])
end
elseif startswith(version_string, "FUJITSU MPI")
Copy link
Member

@giordano giordano Feb 21, 2022

Choose a reason for hiding this comment

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

Ok, this breaks the idea of #542 😄

simonbyrne
simonbyrne commented Feb 21, 2022
View reviewed changes
@vchuravy
Copy link
Member

vchuravy commented Feb 28, 2022

makes it into a new release, is there a way to work around the problem for testing purposes?

Is
https://github.com/EnzymeAD/Enzyme.jl/blob/d774adc69a5c724246f9854b380761cdaba9d80c/test/runtests.jl#L1-L7 what you need?

@vchuravy vchuravy mentioned this pull request Mar 3, 2022
Merged
@vchuravy
Copy link
Member

vchuravy commented Mar 4, 2022

@JuliaRegistrator register subdir=lib/MPIPreferences

@JuliaRegistrator
Copy link

JuliaRegistrator commented Mar 4, 2022

Comments on pull requests will not trigger Registrator, as it is disabled. Please try commenting on a commit or issue.

vchuravy
vchuravy reviewed Mar 4, 2022
View reviewed changes
vchuravy
vchuravy reviewed Mar 4, 2022
View reviewed changes
vchuravy
vchuravy reviewed Mar 4, 2022
View reviewed changes
@vchuravy @giordano
Apply suggestions from code review
612a740 
Co-authored-by: Mosè Giordano <giordano@users.noreply.github.com>
vchuravy
vchuravy reviewed Mar 4, 2022
View reviewed changes
@vchuravy vchuravy mentioned this pull request Mar 6, 2022
Closed
giordano
giordano reviewed Mar 18, 2022
View reviewed changes
@simonbyrne simonbyrne merged commit 890ee6e into master Apr 13, 2022
@simonbyrne simonbyrne deleted the sb/mpipref branch April 13, 2022 05:37
@giordano giordano linked an issue Apr 18, 2022 that may be closed by this pull request
Unknown MPI ABI when using Fujitsu MPI on Fugaku, and segmentation faults when running the tests with this implementation #539
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@giordano giordano giordano left review comments

@vchuravy vchuravy vchuravy approved these changes

+2 more reviewers

@bgoglin bgoglin bgoglin left review comments

@ggouaillardet ggouaillardet ggouaillardet left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Unknown MPI ABI when using Fujitsu MPI on Fugaku, and segmentation faults when running the tests with this implementation

6 participants

@simonbyrne @vchuravy @JuliaRegistrator @giordano @bgoglin @ggouaillardet