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

Deprecate MPI_Graph_create #2

Open
hjelmn opened this issue Mar 6, 2018 · 21 comments
Open

Deprecate MPI_Graph_create #2

hjelmn opened this issue Mar 6, 2018 · 21 comments

Comments

@hjelmn
Copy link

hjelmn commented Mar 6, 2018

Problem

The scalability of MPI_Graph_create() is limited due to the global nature of its arguments. We should deprecate it.

Proposal

Deprecate the following:

int MPI_Graph_create (MPI_Comm comm_old, int nnodes, const int index[],
const int edges[], int reorder, MPI_Comm *comm_graph);

int MPI_Graphdims_get(MPI_Comm comm, int *nnodes, int *nedges);

int MPI_Graph_get(MPI_Comm comm, int maxindex, int maxedges, int index[],
int edges[]);

MPI_GRAPH

Changes to the Text

Add MPI_Graph_create() and associated functions to the deprecated interfaces.

Impact on Implementations

None.

Impact on Users

Update code to the MPI_Dist_graph_create() routine instead.

References

Insert any internal (other issues) or external (websites, papers, etc.) references here.

@hjelmn
Copy link
Author

hjelmn commented Mar 6, 2018

@dholmes-epcc-ed-ac-uk Agreed on this. Here is the proposal.

@dholmes-epcc-ed-ac-uk
Copy link

In addition, MPI_TOPO_TEST has a possible return value of MPI_GRAPH, which should be deprecated.

We should not forget to deprecate MPI_GRAPH_NEIGHBORS_COUNT and MPI_GRAPH_NEIGHBORS (pointing the reader to the "DIST" versions instead).

We should also consider the fate of MPI_GRAPH_MAP. It's parameters are identical to MPI_GRAPH_CREATE and the Advice shows an example usage in terms of its relation to MPI_GRAPH_CREATE. However, there is no "DIST" version for this function. My suggestion would be to define MPI_DIST_GRAPH_MAP (and MPI_DIST_GRAPH_MAP_ADJACENT?) and deprecate MPI_GRAPH_MAP at the same time.

The example implementation of MPI_GRAPH_CREATE (MPI-3.1 pp313-314) is wrong (if used as user code) because the communicator resulting from the MPI_COMM_SPLIT will not have a topology attached to it. However, it indicates the possibility of arguing that MPI_GRAPH_CREATE is not needed because it is syntactic sugar over MPI_COMM_SPLIT (in addition to the scalability argument).

There is text throughout Chapter 7 that refers to MPI_GRAPH and MPI_GRAPH_CREATE. For example, in section 7.6 (MPI-3.1 p314 lines 33-39) when introducing neighbourhood collective operations and discussing the sequence of neighbours. In that example the text already states "use DIST instead" but should be strengthened to "because MPI_GRAPH_CREATE is deprecated".

Should we create and include an example showing how to do the replacement of MPI_GRAPH_CREATE with MPI_DIST_GRAPH_CREATE_ADJACENT? That is, some boiler-plate code that extracts adjacency information from the arguments that would have been given to MPI_GRAPH_CREATE and uses that to form the arguments for MPI_DIST_GRAPH_CREATE_ADJACENT?

@hjelmn
Copy link
Author

hjelmn commented Mar 7, 2018

Yes, I agree. We should probably put together some text showing how to move to dist graph. I will update the issue with the other functions that need to be removed.

BTW, should this go on the main MPI forum issues as well?

@dholmes-epcc-ed-ac-uk
Copy link

dholmes-epcc-ed-ac-uk commented Mar 8, 2018

BTW, should this go on the main MPI forum issues as well?

Yes, I think this is already a topic ready for wider-than-WG discussion/action. We could/should aim to have text ready to formally read at the next face-to-face meeting.

@hjelmn
Copy link
Author

hjelmn commented Mar 8, 2018

Sure. It shouldn't be difficult to get the text ready by June. Should I open the mpi-forum Issue?

@hjelmn
Copy link
Author

hjelmn commented Mar 8, 2018

I am working on the first cut of the text now.

@hjelmn
Copy link
Author

hjelmn commented Mar 8, 2018

Drafts are complete. What is a good way to share them? Should I create a branch on the mpi-standard repo in the mpiwg-coll organization?

@dholmes-epcc-ed-ac-uk
Copy link

@hjelmn Yes, please create a branch in the mpiwg-coll/mpi-standard repo. Do you have sufficient access permissions? If not, @tonyskjellum or @wesbland can authorise you.

@hjelmn
Copy link
Author

hjelmn commented Mar 9, 2018

No, I had myself added to the RMA WG. Forgot that I probably should be added here as well. @wesbland Please add me to mpiwg-coll.

@tonyskjellum
Copy link

tonyskjellum commented Mar 9, 2018 via email

@hjelmn
Copy link
Author

hjelmn commented Mar 14, 2018

Draft text has been uploaded. https://github.com/mpiwg-coll/mpi-standard/tree/deprecate_mpi_graph

@hjelmn
Copy link
Author

hjelmn commented Mar 14, 2018

The ticket number will have to be updated with the final mpi-forum/mpi-issues ticket number once that is up.

The question I have is; is it worth trying to deprecate MPI_Graph in MPI-3.2 (if there is one) or leave it for MPI-4.0? The benefit I see of deprecation in MPI-3.2 is we can potentially remove it in MPI-4.0 and be done with it.

@dholmes-epcc-ed-ac-uk
Copy link

I think usual practice is to deprecate whenever but only remove things in a major version. The most recent deprecation (https://github.com/mpi-forum/mpi-standard/pull/26) was merged after MPI-3.1 so will be included in MPI-Next, whatever that is. We should just aim for as soon as possible!

@hjelmn
Copy link
Author

hjelmn commented Mar 14, 2018

Ok, I will update the text to show as deprecated in MPI-3.2 and we can update if MPI-3.2 does not happen.

@hjelmn
Copy link
Author

hjelmn commented Mar 15, 2018

This is ticket #89.

@hjelmn
Copy link
Author

hjelmn commented Apr 25, 2018

@dholmes-epcc-ed-ac-uk Please let me know when you have had a chance to look at the text changes. I would like to make sure this is ready for discussion at the June meeting.

@tonyskjellum
Copy link

tonyskjellum commented Apr 25, 2018 via email

@hjelmn
Copy link
Author

hjelmn commented Apr 25, 2018

@tonyskjellum
Copy link

tonyskjellum commented Apr 25, 2018 via email

@dholmes-epcc-ed-ac-uk
Copy link

I created pull request https://github.com/mpi-forum/mpi-standard/pull/37 and then added a review to it.

Summary: MPI_GRAPH_MAP is going to cause problems.

@RolfRabenseifner
Copy link

There is no real need to deprecate the MPI-1 virtual graph interface, because it is still an efficient interface for medium size applications. Before deprecating, there should be publications proofing that the implementations of the new dist-graph-create interface are really faster than using the old interface. Whitout any such proof, we should not burden the users with such deprecation. Do such publications exist? For MPICH, OpenMPI, ...?

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

No branches or pull requests

4 participants