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

Add support for directing node-branch sends to PEs #3607

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jszaday
Copy link
Contributor

@jszaday jszaday commented May 10, 2022

This PE adds support for directing sends to particular PEs for node-groups via CkEntryOptions::setNodeGroupPe and/or CkSendMsgNodeBranchPe. Example use-case shown here:
https://gist.github.com/jszaday/486ee195180c42eed2a6f7b14462b123

This was made after an inquiry from Nils Deppe. Couple of general questions:

  • Does this violate any properties of the Charm++ model? Is it too niche?
    • Sanjay: "But... this seems odd breach of abstraction. Nodegroups are meant for work that any pe on the node can do. I think a common pattern is to always have a group associated with a nodegroup.So using a group may be a better solution. No?"
  • Does this functionality already exist and we missed it?

TODOs:

  • Decide on names for all added methods.
  • Decide whether we need support for inline/immediate sends via this new procedure?
  • Update documentation.

// TODO ( expand support for these options at some point... )
CkAssertMsg(!((opts & CK_MSG_INLINE) || (opts & CK_MSG_IMMEDIATE)),
"pe-level inline and immediate sends to node branchs unsupported");
int numPes = (pe == CK_PE_ALL) ? CkNumPes() : 1;
Copy link
Contributor Author

@jszaday jszaday May 10, 2022

Choose a reason for hiding this comment

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

Maybe this should fail if given CK_PE_ALL?

Alternatively, should it send to all PEs? If so, this would currently break since ...ClEnqueue only performs node broadcasts.

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

1 participant