search

mpiwg-coll / coll-issues

Repository for internal Collectives Working Group issues and discussions
2 stars 0 forks source link

Deprecate MPI_Graph_create #2

Open hjelmn opened 6 years ago

hjelmn commented 6 years ago

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 commented 6 years ago

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

dholmes-epcc-ed-ac-uk commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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

hjelmn commented 6 years ago

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

hjelmn commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

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 commented 6 years ago

Will do

Anthony Skjellum, PhD 205-807-4968

On Mar 9, 2018, at 1:28 PM, Nathan Hjelm notifications@github.com wrote:

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.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

hjelmn commented 6 years ago

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

hjelmn commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

This is ticket #89.

hjelmn commented 6 years ago

@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 commented 6 years ago

Is this somewhere related to the ticket ? Is there a link to pull request. ?

Anthony Skjellum, PhD 205-807-4968

On Apr 24, 2018, at 5:44 PM, Nathan Hjelm notifications@github.com wrote:

@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.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

hjelmn commented 6 years ago

@tonyskjellum Its in a branch. No PR yet. Branch: https://github.com/mpiwg-coll/mpi-standard/tree/deprecate_mpi_graph

tonyskjellum commented 6 years ago

Thank you

Anthony Skjellum, PhD 205-807-4968

On Apr 24, 2018, at 6:05 PM, Nathan Hjelm notifications@github.com wrote:

@tonyskjellum Its in a branch. No PR yet. Branch: https://github.com/mpiwg-coll/mpi-standard/tree/deprecate_mpi_graph

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

dholmes-epcc-ed-ac-uk commented 6 years ago

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 commented 5 years ago

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, ...?