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

Teams: allow split stride of zero iff size is 1 #1136

Merged
merged 4 commits into from
Jun 18, 2024

Conversation

davidozog
Copy link
Member

This PR allows team split operations to have a stride of zero if and only if the new team size is 1.

@davidozog davidozog added this to the v1.5.3 milestone Jun 17, 2024
@davidozog davidozog self-assigned this Jun 17, 2024
src/shmem_team.c Outdated Show resolved Hide resolved
Copy link
Collaborator

@wrrobin wrrobin left a comment

Choose a reason for hiding this comment

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

LGTM. Minor change to update team->stride to 0 when PE_stride is ignored with PE-size = 1 can be done.

Also, if split size is 1, internally set stride to 0
@davidozog davidozog requested a review from wrrobin June 17, 2024 19:08
@davidozog
Copy link
Member Author

davidozog commented Jun 18, 2024

@wrrobin @philipmarshall21 - I reverted the zero-stride checks across collectives (998f7c9) by forcing stride=1 internally, which I think is a little cleaner. Can you please re-review?

(Also this updates the SOS tests-sos submodule to point to the latest commit).

Copy link
Collaborator

@wrrobin wrrobin left a comment

Choose a reason for hiding this comment

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

LGTM. So, internally, we are not allowing stride to be 0. Perhaps, we should discuss if we need to revisit this in OpenSHMEM.

@davidozog
Copy link
Member Author

LGTM. So, internally, we are not allowing stride to be 0. Perhaps, we should discuss if we need to revisit this in OpenSHMEM.

Yes, agreed. Fortunately I think the spec is arguably well-defined if the following is true: shmem_team_split_strided allows stride to be zero only if size is 1 (but stride can also be any value when size is 1). So it's up to library implementers to handle the special zero-stride case, because it's likely to lead to divide-by-zero exceptions.

I've also drafted a change here that should help clarify this in the spec:
davidozog/openshmem-specification@010e424

I will propose it for v1.6 if there's enough time...

Copy link
Collaborator

@philipmarshall21 philipmarshall21 left a comment

Choose a reason for hiding this comment

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

LGTM!

@davidozog davidozog merged commit f3689bb into Sandia-OpenSHMEM:main Jun 18, 2024
72 checks passed
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.

None yet

3 participants