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

Integration of Spaces with Teams/Contexts #361

Closed
jdinan opened this issue Feb 6, 2020 · 5 comments
Closed

Integration of Spaces with Teams/Contexts #361

jdinan opened this issue Feb 6, 2020 · 5 comments

Comments

@jdinan
Copy link
Collaborator

jdinan commented Feb 6, 2020

This ISSUE describes the various options that can be used to link a context to a particular team and space. Now, the default shmem_ctx_create creates a context with a set of options, and shmem_team_create_ctx creates a context with a set of options on a particular team.

How do we handle when we introduce space in OpenSHMEM-1.6? Do we introduce a new API, break compatibility and fix the existing APIs? Or do we need to make the current APIs extendable? Or do we fix the current APIs?

Different API options available for us are listed below from the Feb/6/2020 F2F discussion:

Current (OpenSHMEM-1.5 Draft):

int shmem_ctx_create(long options, shmem_ctx_t *ctx);
int shmem_team_create_ctx(shmem_team_t team, long options, shmem_ctx_t *ctx);

/*** BELOW REQUIRE CHANGES in OpenSHMEM 1.5 ***/

Option_1:

(a) int shmem_ctx_create(long options, shmem_ctx_t *ctx); /* No ABI breakage */
(b) int shmem_ctx_create(long options, shmem_ctx_t *ctx, ...); /* Break ABI now */
int shmem_team_create_ctx(shmem_team_t team, long options, shmem_ctx_t *ctx, ...);

Option_2:

// Want mask to avoid passing bits to shmem_ctx_create that aren't valid in that function

int shmem_ctx_create(long options, shmem_ctx_t *ctx);
int shmem_team_create_ctx(shmem_team_t team, long options, shmem_ctx_t *ctx, long mask, const shmem_ctx_opt_t *opt);

Option_5:

int shmem_ctx_create(long options, shmem_ctx_t *ctx);
int shmem_ext_ctx_create(long options, shmem_ctx_t *ctx, shmem_ctx_opt_t opt); /* Add in 1.5 */

/*** BELOW CAN BE DONE WITH CHANGES ONLY IN 1.6 ***/

Option_3:

// Breaks ABI (not API) backward compabitility

int shmem_ctx_create(long options, shmem_ctx_t *ctx, ...);
int shmem_team_create_ctx(shmem_team_t team, long options, shmem_ctx_t *ctx, ...);

Option_4:

// Don't need mask, can reuse options in this case

int shmem_ctx_create(long options, shmem_ctx_t *ctx);
int shmem_team_create_ctx(shmem_team_t team, long options, shmem_ctx_t *ctx);
int shmem_ext_ctx_create(long options, shmem_ctx_t *ctx, shmem_ctx_opt_t opt); /* Add in 1.6 */

Option_6:

options += SHMEM_CTX_NOSPACE /* 1.6 */

int shmem_ctx_create(long options, shmem_ctx_t *ctx);
int shmem_team_create_ctx(shmem_team_t team, long options, shmem_ctx_t *ctx);
int shmem_ctx_space_attach(shmem_ctx_t ctx, shmem_space_t space); /* 1.6 */
@naveen-rn
Copy link
Contributor

Vote:1 Do we need to fix now?

Vote Count
Fix Now (either option:1(a), option:1(b), option:2, option:5) 0
Fix Later (either option:3, option:4, option:6) 0

Vote:2 Select a particular Fix, if vote:1 passes with Fix Now:

Vote Count
Option:1 (a) 0
Option:1 (b) 0
Option:2 0
Option:5 0

Vote:3 Select a particular Fix for Fix Later:

No need to select the option now, we can do that later.

@jdinan
Copy link
Collaborator Author

jdinan commented Feb 10, 2020

@naveen-rn If I understood correctly, the preference from last week was to implement Option 2 for OpenSHMEM 1.5:

int shmem_ctx_create(long options, shmem_ctx_t *ctx);
int shmem_team_create_ctx(shmem_team_t team, long options, shmem_ctx_t *ctx, long mask, const shmem_ctx_opt_t *opt);

@jdinan
Copy link
Collaborator Author

jdinan commented Feb 25, 2020

@naveen-rn What's the status of this issue? Are we adding the above for OpenSHMEM 1.5?

@jdinan
Copy link
Collaborator Author

jdinan commented Feb 25, 2020

@manjugv Is still having mixed thoughts on this approach.

@naveen-rn
Copy link
Contributor

closing this issue, as it is handled as part of #455.

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

No branches or pull requests

2 participants