search

openshmem-org / specification

OpenSHMEM Application Programming Interface
http://www.openshmem.org
49 stars 32 forks source link

Clarify return value for team split on SHMEM_TEAM_INVALID #461

Closed davidozog closed 2 years ago

davidozog commented 3 years ago

This PR is meant to address #450 (as well as https://github.com/Sandia-OpenSHMEM/SOS/issues/969 and https://github.com/Sandia-OpenSHMEM/SOS/issues/972).

I don't believe https://github.com/openshmem-org/specification/commit/8b44a29f6075a666aa3fcb3a9d123d1e808c0ec8 is a semantic change - just a clarification that a split operation on a SHMEM_TEAM_INVALID parent is indeed considered "unsuccessful" and the routine should return a nonzero value.

I also included a small change to the 2D team split example code (https://github.com/openshmem-org/specification/commit/ec66ff256fe78c9dba04afdb90b23bd02e66f22b). This has no effect on the example, but leaving these variables uninitialized commonly causes a compiler warning. Please let me know if you prefer this in a separate PR...

davidozog commented 3 years ago

@jdinan and @manjugv - should this ticket have an official reading? I think it's not needed, but am happy to do it.

I also just realized the https://github.com/openshmem-org/specification/commit/ec66ff256fe78c9dba04afdb90b23bd02e66f22b commit is already approved in #449 but was never merged.

jdinan commented 3 years ago

@davidozog Thanks for keeping track of this ticket. This feels more significant than a doc edit to me. Given that we have the time, I would prefer to read this and give the committee a chance to weigh in on whether the case that's being clarified is one where the function should return nonzero.

davidozog commented 3 years ago

@jdinan - sure. Maybe I can do an unofficial reading at the upcoming meeting to see if it generates any concerns. It's too late to announce an official reading this time...

davidozog commented 3 years ago

Note - this should go into errata (and/or pending errata?)

Also need a changelog entry.

davidozog commented 2 years ago

@jdinan - I was about to cherry-pick 503317b96356cca794538516d604b526bb026aea from @kholland-intel's PR, which adds a v1.6 change-log that mentions shmem_team_ptr. Is that ok, or would you prefer a separate change-log commit on this PR that might conflict later?

jdinan commented 2 years ago

@davidozog Please go ahead and add a changelog entry to the effect of "Clarified the return value of team split operations when the parent team is SHMEM_TEAM_INVALID". In the future, I'd prefer to include the changelog entry with the reading since it will help with consistency.

Please don't cherry pick from another PR. I'll deal with any merge conflicts as they come up.

jdinan commented 2 years ago

@davidozog The changelog heading in question was merged into master. Please merge from master and add your changelog entry.

jdinan commented 2 years ago

@davidozog Could you please review the changelog entry in 75559f8?

davidozog commented 2 years ago

@jdinan - 75559f8 looks great, and thanks for merging this with master. And I'm so sorry for the delay!

davidozog commented 2 years ago

@jdinan - I believe this PR is ready to merge. Is there anything pending on your end?

(This should close https://github.com/Sandia-OpenSHMEM/SOS/issues/969)