Skip to content

MPI_Comm cannot expected to be a pointer #1799

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Mar 5, 2025
Merged

MPI_Comm cannot expected to be a pointer #1799

merged 5 commits into from
Mar 5, 2025

Conversation

pratikvn
Copy link
Member

Fixes an issue where it was expected that MPI_Comm is comparable to a pointer. But some implementations (like MPICH) just use a reference to an int and hence are not comparable to std::nullptr_t

@pratikvn pratikvn added the is:bugfix This fixes a bug label Feb 28, 2025
@pratikvn pratikvn requested a review from MarcelKoch February 28, 2025 16:52
@pratikvn pratikvn self-assigned this Feb 28, 2025
@ginkgo-bot ginkgo-bot added the mod:core This is related to the core module. label Feb 28, 2025
@pratikvn
Copy link
Member Author

I am actually not sure why the first if check is needed (in is_identical and in is_congruent. Maybe @MarcelKoch can elaborate on this ?

@pratikvn pratikvn changed the title WIP: MPI_Comm cannot be expected to be pointer MPI_Comm cannot expected to be a pointer Mar 2, 2025
MarcelKoch
MarcelKoch reviewed Mar 3, 2025
View reviewed changes
@yhmtsai
Copy link
Member

yhmtsai commented Mar 3, 2025

what is the empty state of MPICH? 0 here?

@pratikvn
Copy link
Member Author

pratikvn commented Mar 4, 2025

@yhmtsai , yes, empty state is NULL, or 0.

@pratikvn pratikvn added 1:ST:run-full-test 1:ST:no-changelog-entry Skip the wiki check for changelog update labels Mar 4, 2025
yhmtsai
yhmtsai approved these changes Mar 4, 2025
View reviewed changes
Copy link
Member

@yhmtsai yhmtsai left a comment

Choose a reason for hiding this comment

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

some typos are caught by spell-check. could you also change those?

@pratikvn pratikvn added the 1:ST:ready-to-merge This PR is ready to merge. label Mar 5, 2025
@pratikvn pratikvn merged commit 936f87c into develop Mar 5, 2025
9 of 11 checks passed
@pratikvn pratikvn deleted the fix/mpi-bug branch March 5, 2025 17:43
@sonarqubecloud SonarQubeCloud
Copy link

sonarqubecloud bot commented Mar 6, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MarcelKoch MarcelKoch MarcelKoch approved these changes

@yhmtsai yhmtsai yhmtsai approved these changes

Assignees

@pratikvn pratikvn

Labels

1:ST:no-changelog-entry Skip the wiki check for changelog update 1:ST:ready-to-merge This PR is ready to merge. 1:ST:run-full-test is:bugfix This fixes a bug mod:core This is related to the core module.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@pratikvn @yhmtsai @MarcelKoch @ginkgo-bot