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 edits from September F2F #302

Merged
merged 9 commits into from
Dec 10, 2019
Merged

Teams edits from September F2F #302

merged 9 commits into from
Dec 10, 2019

Conversation

jdinan
Copy link
Collaborator

@jdinan jdinan commented Oct 9, 2019

Edits captured on #282 moved here for section committee review

@jdinan jdinan requested review from gmegan, wrrobin and agrippa October 9, 2019 14:18
@jdinan
Copy link
Collaborator Author

jdinan commented Oct 9, 2019

@agrippa @wrrobin @gmegan Please review this PR for merging as a section committee edit. Merge requires unanimous approval by all section committee members.

@jdinan jdinan self-assigned this Oct 9, 2019
@jdinan jdinan added this to the OpenSHMEM 1.5 milestone Oct 9, 2019
agrippa
agrippa reviewed Oct 16, 2019
View reviewed changes
content/shmem_collect.tex Outdated Show resolved Hide resolved
@jdinan jdinan requested a review from agrippa October 29, 2019 19:38
@jdinan
Copy link
Collaborator Author

jdinan commented Nov 7, 2019

@wrrobin @gmegan Can we please wrap this up?

@jdinan
Copy link
Collaborator Author

jdinan commented Nov 12, 2019

@gmegan Need your approval before this PR can be merged. Can you please review?

gmegan
gmegan approved these changes Nov 14, 2019
View reviewed changes
Copy link
Collaborator

@gmegan gmegan left a comment

Choose a reason for hiding this comment

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

This looks good as is. I left some rewording suggestions as options.

content/shmem_collect.tex Outdated Show resolved Hide resolved
content/shmem_sync.tex Outdated Show resolved Hide resolved
content/teams_intro.tex Outdated Show resolved Hide resolved
@jdinan
Copy link
Collaborator Author

jdinan commented Nov 15, 2019

@agrippa @wrrobin @gmegan Please review changes from @gmegan in fe0ad7b.

manjugv
manjugv reviewed Nov 15, 2019
View reviewed changes
content/shmem_sync.tex Outdated Show resolved Hide resolved
James Dinan added 2 commits November 15, 2019 13:23
Fix shmem_sync_example
e5fe070 
This example was broken in several ways.  The team parameters were
incorrect (e.g. odd_npes led to broken behavior) and also PE 0 was
included in both the twos and threes, leading to data corruption.
Replace PEs with \acp{PE}
2e5bb54
@jdinan
Copy link
Collaborator Author

jdinan commented Nov 15, 2019

Hi All, please re-review shmem_sync_example.c changes. I ran this against our teams implementation and found more errors than we initially realized. New changes pushed in e5fe070 and the updated example runs successfully with any number of PEs.

@jdinan jdinan mentioned this pull request Nov 15, 2019
Merged
10 tasks
@davidozog davidozog mentioned this pull request Nov 19, 2019
Merged
Fix shmem_sync_example
78d735b 
Signed-off-by: James Dinan <[email protected]>
@jdinan
Copy link
Collaborator Author

jdinan commented Dec 4, 2019

I apologize for the churn on this PR. The example_code/shmem_sync_example.c example is now fixed and tested on the SOS implementation of teams (available with shmemx_* prefix on SOS master). There was an additional race on x for PEs whose IDs are a multiple of both 2 and 3, which is fixed by adding a sync on SHMEM_TEAM_WORLD between updates.

Would appreciate a final affirmative from each section committee member so that I can finally close out this edit. Thanks!

agrippa
agrippa reviewed Dec 5, 2019
View reviewed changes
content/teams_intro.tex Outdated Show resolved Hide resolved
Minor edit from @agrippa
0408da8 
Signed-off-by: James Dinan <[email protected]>
@agrippa
Copy link
Collaborator

agrippa commented Dec 5, 2019

LGTM, thanks Jim!

@jdinan jdinan merged commit 71a03f6 into openshmem-org:master Dec 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@naveen-rn naveen-rn naveen-rn left review comments

@nspark nspark nspark left review comments

@manjugv manjugv manjugv left review comments

@agrippa agrippa agrippa approved these changes

@gmegan gmegan gmegan approved these changes

@wrrobin wrrobin wrrobin approved these changes

Assignees

@jdinan jdinan

Labels
SectionCommittee
Projects
No open projects
OpenSHMEM 1.5rc1 Section Committees
Status: Teams and Contexts (Sec. 9.4-9.5)
Milestone
OpenSHMEM 1.5
Development

Successfully merging this pull request may close these issues.
7 participants
@jdinan @agrippa @manjugv @nspark @wrrobin @naveen-rn @gmegan