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.

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

Fix communicator categorization #1645

Merged
merged 4 commits into from
Jul 18, 2024
Merged

Fix communicator categorization #1645

merged 4 commits into from
Jul 18, 2024

Conversation

thoasm
Copy link
Member

@thoasm thoasm commented Jul 10, 2024

The default for is_communicator_group was std::true_type for some reason, which means that every type you pass in was considered a communicator.
This PR fixes it (so it defaults to std::false_type) and adds tests to prevent this from happening again.

@thoasm thoasm added reg:testing This is related to testing. mod:cuda This is related to the CUDA module. 1:ST:ready-for-review This PR is ready for review mod:hip This is related to the HIP module. mod:dpcpp This is related to the DPC++ module. is:bugfix This fixes a bug 1:ST:no-changelog-entry Skip the wiki check for changelog update labels Jul 10, 2024
@thoasm thoasm force-pushed the fix_communicator_detection branch 2 times, most recently from dadb94e to 7ad4e69 Compare July 10, 2024 16:13
yhmtsai
yhmtsai approved these changes Jul 11, 2024
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.

LGTM, but need to reformat the files

@thoasm thoasm force-pushed the fix_communicator_detection branch from 7ad4e69 to 0d2a292 Compare July 11, 2024 12:15
@thoasm
Copy link
Member Author

thoasm commented Jul 11, 2024

format!

@thoasm thoasm force-pushed the fix_communicator_detection branch from 5a9dbc9 to e470325 Compare July 11, 2024 13:10
@thoasm thoasm self-assigned this Jul 11, 2024
@thoasm thoasm added 1:ST:ready-to-merge This PR is ready to merge. and removed 1:ST:ready-for-review This PR is ready for review labels Jul 11, 2024
@thoasm thoasm force-pushed the fix_communicator_detection branch from e470325 to 14f0e24 Compare July 18, 2024 13:11
@thoasm thoasm merged commit 6ed7108 into develop Jul 18, 2024
10 of 14 checks passed
@thoasm thoasm deleted the fix_communicator_detection branch July 18, 2024 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@upsj upsj upsj approved these changes

@MarcelKoch MarcelKoch MarcelKoch approved these changes

@yhmtsai yhmtsai yhmtsai approved these changes

Assignees

@thoasm thoasm

Labels
1:ST:no-changelog-entry Skip the wiki check for changelog update 1:ST:ready-to-merge This PR is ready to merge. is:bugfix This fixes a bug mod:cuda This is related to the CUDA module. mod:dpcpp This is related to the DPC++ module. mod:hip This is related to the HIP module. reg:testing This is related to testing.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@thoasm @upsj @MarcelKoch @yhmtsai @ginkgo-bot