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

Teams API topic branch #886

Merged
merged 99 commits into from
Nov 19, 2019
Merged

Teams API topic branch #886

merged 99 commits into from
Nov 19, 2019

Conversation

davidozog
Copy link
Member

@davidozog davidozog commented Sep 20, 2019 •
edited
Loading

This is a work-in-progress branch that implements the OpenSHMEM v1.5 teams API. This pull request is intended to merge the wip/teams branch from my fork into the topic/teams branch into the SOS main repository.

Here is a running checklist of open questions and TODO items:

  • Add more psync arrays for back-to-back collectives
  • The unit tests are more-or-less consistent with the teams proposal examples, but may need modification (most require multiple PEs).
  • Write unit test(s) with more complete coverage of the teams API and various types.
  • Support more than 64 teams.
  • Are some types missing from shmem_internal_op.h?
  • Should I add pWrk space for each team at initialization? I deferred this since SOS doesn't yet make use of pWrk arrays.
  • I'm not sure whether shmem_internal_1st_nonzero_bit supports big-endiannass.
  • The C11/C++ "extras" bindings might need some extra help.
  • SHMEM_TEAM_SHARED only works with a uniform stride across the local process IDs. Users can otherwise set DISABLE_TEAM_SHARED, but this could definitely be cleaner.
  • Whoops - are private contexts lost during team destroy? (test it)

davidozog and others added 30 commits April 12, 2019 17:44
@davidozog
Add Teams bare-bones API without any functionality
6d3a31b 
Signed-off-by: David M. Ozog <[email protected]>
@davidozog
Add a KVS of available teams (DO NOT MERGE)
73439c9 
Signed-off-by: David M. Ozog <[email protected]>
@davidozog
Teams: remove uthash and make all strides linear
99d2f97 
Signed-off-by: David M. Ozog <[email protected]>
@davidozog
Add shmemx_tem_translate unit test
f00ac33 
Signed-off-by: David M. Ozog <[email protected]>
@davidozog
Fix global_exit seg fault with no backtrace method
1526cf2 
Signed-off-by: David Ozog <[email protected]>
@davidozog
Return #include "config.h" to shmem_env.h
2f2fb84 
Signed-off-by: David M. Ozog <[email protected]>
@davidozog
Assure teams API routines are prepended w/ shmemx
52339d8 
Signed-off-by: David Ozog <[email protected]>
@davidozog
Add implementation/test of shmem_team_translate_pe
0e9a662 
Signed-off-by: David M. Ozog <[email protected]>
@davidozog
Update to shmem_internal_team_t where needed
1b53db4 
Signed-off-by: David M. Ozog <[email protected]>
@davidozog
Implement shmemx_team_split_2D and add a unit test
81884da 
Signed-off-by: David M. Ozog <[email protected]>
@davidozog
Cleanup team split code, remove post-split barrier
23226ca 
Signed-off-by: David M. Ozog <[email protected]>
@davidozog
Add shmemx_sync/shmemx_team_sync with unit test
8e4f844 
Signed-off-by: David M. Ozog <[email protected]>
@davidozog
Implement shmemx_broadcast with teams support
41feee7 
Signed-off-by: David M. Ozog <[email protected]>
@davidozog
Add teams collect, fcollect, alltoall, alltoalls
e587b63 
Signed-off-by: David M. Ozog <[email protected]>
@davidozog
Add bindings for collectives, fix c11 shmemx tests
3332081 
Signed-off-by: David M. Ozog <[email protected]>
@davidozog
Cleanup the shmemx tests, don't exit(0) on failure
b01b801 
Signed-off-by: David M. Ozog <[email protected]>
@davidozog
Point contexts to the appropriate team, add test
18becf8 
Signed-off-by: David Ozog <[email protected]>
@davidozog
Add team PE indexing for ctx RMA/AMO operations
b7fd4fa 
Signed-off-by: David M. Ozog <[email protected]>
@davidozog
Add C++/C11 teams API bindings and set psync bits
1ef41d7 
Signed-off-by: David M. Ozog <[email protected]>
@davidozog
Rename some vars and fix destroy & op_to_all bugs
4c8fa5c 
Signed-off-by: David Ozog <[email protected]>
@davidozog
Add team field to portals and none transport ctxs
8c73f23 
Signed-off-by: David M. Ozog <[email protected]>
@davidozog
Support reduce_local unsigned types, fix example
1598f51 
Signed-off-by: David Ozog <[email protected]>
@davidozog
Use separate psync in non-barrier/sync collectives
4a1391c 
Signed-off-by: David M. Ozog <[email protected]>
@davidozog
Add preliminary support for SHMEMX_TEAM_SHARED
568324b 
Signed-off-by: David M. Ozog <[email protected]>
@davidozog
Rewrite TEAM_SHARED stride check without a malloc
00bac21 
Signed-off-by: David Ozog <[email protected]>
@davidozog
Fix pWrk mistake, rename a variable in team init
2665b71 
Signed-off-by: David Ozog <[email protected]>
@davidozog
Simplify search for local peer PEs in TEAM_SHARED
63cfde3 
Signed-off-by: David Ozog <[email protected]>
@davidozog
Merge branch 'master' of github.com:davidozog/sandia-shmem into wip/t…
de5ba2c 
…eams
@davidozog
Cleaning pre-defined teams initialization
a4f0217 
Signed-off-by: David Ozog <[email protected]>
@davidozog
Cleanup teams c11/cxx bindings and SHMEMX_* fixes
41109d2 
Signed-off-by: David M. Ozog <[email protected]>
James Dinan added 4 commits November 16, 2019 08:40
@davidozog davidozog mentioned this pull request Nov 16, 2019
Closed
@davidozog
Various teams review, round #4.
daff427 
* README update
* teams_fini reordering
* shmem_internal_free moved
* added shmem_internal_bit_fetch
* rename/comment check_for_linear_stride
* move teams_init errors to appropriate place
* move teams_init pool initializations to appropriate place
* use shmem_internal_params.TEAMS_MAX
* use asserts where needed
* fix 'myteam' leak in parent-only PEs
* fix choose_psync bug for parent-only PEs
* more informative error in split_strided
* All of parent must call split_strided in split_2d
* cleanup unit tests
* other minor changes...

Signed-off-by: David M. Ozog <[email protected]>
@davidozog
Merge pull request #8 from jdinan/wip/teams
9044877 
Fixes to wip/teams
@davidozog
Merge branch 'wip/teams' of github.com:davidozog/sandia-shmem into wi…
595d1f0 
…p/teams
@davidozog
Destroy the predefined teams in shmem_team_fini
57d414b 
Signed-off-by: David M. Ozog <[email protected]>
@davidozog
Destroy predefined teams outside portals as well
7c57f6f 
Signed-off-by: David M. Ozog <[email protected]>
jdinan
jdinan reviewed Nov 18, 2019
View reviewed changes
src/shmem_team.h Outdated Show resolved Hide resolved
@jdinan
Copy link
Member

jdinan commented Nov 18, 2019

@davidozog Just a few minor comments for myself while reviewing your most recent patches. I will work through these items (file issues, push minor changes, etc.) and expect this to be ready to merge later today. With respect to the tests from the spec, I will move these to a separate PR to allow more time for us to clean them up and propose fixes upstream in the spec.

James Dinan added 8 commits November 18, 2019 13:06
Remove redundant PE id from error message
d40b74b 
Signed-off-by: James Dinan <[email protected]>
Improve error handling for SHMEM_TEAMS_MAX
1d2da2b 
Catch and correct for SHMEM_TEAMS_MAX values that are too small.

Signed-off-by: James Dinan <[email protected]>
Only set bits corresponding to allocated psyncs
b4e26ca 
Signed-off-by: James Dinan <[email protected]>
Add psync avail bit string debugging output
3b85c1a 
Signed-off-by: James Dinan <[email protected]>
Add comment about SHMEM_CTX_DEFAULT destroy
24ca0a0 
This is a special case; the default context isn't inserted into the
contexts list on the world team, so it is not freed when the teams are
destroyed.  This context must be freed separately by the transport
layer.

Signed-off-by: James Dinan <[email protected]>
Remove sketchy spec tests
90886c8 
These tests are failing and it looks like there may be bugs in the unit
tests.  Setting these tests aside for debugging.

Signed-off-by: James Dinan <[email protected]>
Set my_pe field on new teams
c76260f 
Signed-off-by: James Dinan <[email protected]>
Fix bugs in spec tests
0310b31 
Signed-off-by: James Dinan <[email protected]>
@jdinan
Copy link
Member

jdinan commented Nov 18, 2019

@davidozog I took the liberty of pushing directly to your fork to speed up time to merge. Hope you don't mind! Please review the edits and, if all looks good on your end, go ahead and merge! Excellent work on a huge new feature! 💯

I was able to sort out some, but not all of the issues with the spec tests. After merging, please git revert 90886c8 and post a new PR so we can work through the remaining issues with those tests.

jdinan
jdinan approved these changes Nov 18, 2019
View reviewed changes
Copy link
Member

@jdinan jdinan left a comment •
edited
Loading

Choose a reason for hiding this comment

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

🥇 :shipit:

@jdinan
Copy link
Member

jdinan commented Nov 18, 2019

Sorry, one last patch. Was looking into the other build warnings and realized we updated from int nreduce on the legacy reductions to size_t nreduce on the teams reductions. Fixing those build warnings fixed our support for the larger type.

@jdinan jdinan mentioned this pull request Nov 18, 2019
Closed
@davidozog davidozog merged commit 788ad10 into Sandia-OpenSHMEM:topic/teams Nov 19, 2019
@jdinan jdinan mentioned this pull request Nov 19, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wrrobin wrrobin wrrobin left review comments

@jdinan jdinan jdinan approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@davidozog @jdinan @wrrobin