Skip to content

Conversation

davidozog
Copy link
Collaborator

This is a continuation of the Bundles proposal, so closes the #481 pull request. Sessions are a generalization of Bundles, so users can pass hints for context optimization that go beyond just chaining/bundling. There is more discussion on this API design in the original ticket, #189.

As usual, I appreciate any feedback - thanks in advance!

@davidozog
Copy link
Collaborator Author

Also, for those interested, an example of a bare-minimum implementation of this proposed API in Sandia OpenSHMEM here:
Sandia-OpenSHMEM/SOS#1065

It simply ignores all hints...

@davidozog
Copy link
Collaborator Author

I address some key suggestions from the unofficial reading on Sept. 30th with commit 93f510e. Thanks for the feedback!

Here are a few other notes from that meeting:

  • There was another request for an interface to provide values to hints, such as the number of upcoming (chainable) RMA operations. Something like an "epoch" sub-session or a "get/set" API for existing library constants. This was suggested by Brandon Potter at AMD (but don't know his Github handle... does @khamidouche?). Thoughts?
  • Concern was raised that some implementations will not be able to support sufficiently long chains ("postlists") to see any benefit from the chaining hint in sessions. Did I capture that correctly @manjugv / @naveen-rn?

If anybody feels I missed something, please let me know! I unfortunately lost my PDF notes in a laptop crash, so I'm going by memory...

@davidozog
Copy link
Collaborator Author

davidozog commented Feb 13, 2024

Special ballot diff:
6417925..6d0f714

(cancelled)

@davidozog
Copy link
Collaborator Author

Special ballot diff:
aa84edeb..e22a3f15

\ChangelogRef{teamreducetypes}%
%
\item Added the session routines, \FUNC{shmem\_session\_start} and
\FUNC{shmem\_session\_stop}, which allow users to pass hints to the
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should these start/stop functions have been updated to include ctx as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oops, yes; I think these should have been renamed just like every other instance below.

How shall we handle this @jdinan? Do you think it's eligible for a doc edit?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants