Add teams spec tests to "spec-example" directory #918
Conversation
Signed-off-by: David M. Ozog <[email protected]>
davidozog
changed the title
|
Thanks for separating these out. Now we just need to fix the bugs in these tests. 😬 |
Signed-off-by: David M. Ozog <[email protected]>
Signed-off-by: David Ozog <[email protected]>
Signed-off-by: David M. Ozog <[email protected]>
Signed-off-by: David M. Ozog <[email protected]>
|
|
||
| shmem_finalize(); | ||
| return 0; | ||
| } |
There was a problem hiding this comment.
On the upside, it looks like it will work now, which is great.
| shmemx_team_split_strided(SHMEMX_TEAM_WORLD, 0, 3, (npes + 2) / 3, &conf, cmask, &team_3); | ||
|
|
||
| /* Create a context on team_2. */ | ||
| int ret = shmemx_team_create_ctx(team_2, 0, &ctx_2); |
There was a problem hiding this comment.
If team creation fails, we will end up passing SHMEM_TEAM_INVALID to context creation.
There was a problem hiding this comment.
Should we check this and set the context to invalid if the team creation fails?
There was a problem hiding this comment.
I think this depends on the behavior of shmem_team_create_ctx when it's passed SHMEM_TEAM_INVALID, which is apparently not quite specified (openshmem-org/specification#324).
If shmem_team_create_ctx returns nonzero when passed SHMEM_TEAM_INVALID, then the context will be set to invalid here, correct? In that case, I'm not sure we would need a check for SHMEM_TEAM_INVALID here, at least after openshmem-org/specification#305 is ratified. (Side question: does a nonzero return value from shmem_split_strided imply the returned team objects are SHMEM_TEAM_INVALID?) Perhaps I'm missing something? If passing SHMEM_TEAM_INVALID to shmem_team_create_ctx is an error or undefined, then that's obviously a different story...
|
|
||
| /* Sum the values among PEs that are in both team_2 and team_3 on PE 0 with ctx_2. */ | ||
| if (team_3 != SHMEMX_TEAM_INVALID && team_2 != SHMEMX_TEAM_INVALID) | ||
| shmem_ctx_int_atomic_add(ctx_2, &sum, val_2 + val_3, 0); |
There was a problem hiding this comment.
This is an unreliable way to check, since context creation failure is local (not global)...
There was a problem hiding this comment.
Whoops - right. So I just need to include a check for ctx_2 != SHMEM_CTX_INVALID right? The test only wants PEs that are in both teams to do this atomic.
|
I've recently made changes to these examples that should address all the issues discussed here. @jdinan, would you mind if I pushed those changes to this PR and then re-review? And I'm assuming you are still able to do the review? (I'd bet it'll be real easy since you probably reviewed this upstream.) |
|
Sure, go for it. |
Signed-off-by: David M. Ozog <[email protected]>
|
@jdinan - I merge the upstream changes to these example programs. They are pretty much the same, with the usual |
|
Sure, assuming the functions in question are not intended by the example to be externally callable, can you put the |
|
@jdinan I posted the doc-edit and this is ready for re-review, thanks! |
This is a spin-off PR of #907, which now adds the teams example programs as tests in the
spec-exampledirectory.Currently, the tests/examples are not passing due to the issues mentioned in #907. Minor modifications have been made (
shmem->shmemx) so that the programs compile with SOS, but they are otherwise very similar to the upstream versions (including filenames).