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

Use of [] in output arrays #125

Closed
mpiforumbot opened this issue Jul 24, 2016 · 28 comments
Closed

Use of [] in output arrays #125

mpiforumbot opened this issue Jul 24, 2016 · 28 comments

Comments

@mpiforumbot
Copy link
Collaborator

mpiforumbot commented Jul 24, 2016

Originally by gropp on 2009-02-10 18:49:52 -0600


Description

It was noticed that the bindings do not consistently use [] or * for input arrays (many in the Forum expressed a preference for the [] form for input arrays; e.g., const int recvcounts[] to make clear that the input is an array that is not changed; the const is included because this is better style for in-only arguments such as this). The question was raised about output arrays and the use of [] in output arguments. This ticket is a place holder for the chapter authors to inspect their chapters for such usage (e.g., output arrays) and then to propose any changes for consistency. Chapter authors should edit this description to indicate that their chapter has been checked.

Ticket #126 covers input arrays

Proposed Solution

Chapter 1 (Introduction)

Chapter 2 (Terms and Conventions)

Chapter 3 (Pt2pt)

Chapter 4 (Datatypes)

Chapter 5 (Collective Communication)

No issues

Chapter 6 (Groups, Contexts, Communicators, and Caching)

p. 215, line 12, Replace

#!c
int MPI_Group_translate_ranks (MPI_Group group1, int n, int *ranks1, 
                               MPI_Group group2, int *ranks2)

by

#!c
int MPI_Group_translate_ranks (MPI_Group group1, int n, int ranks1[],
                               MPI_Group group2, int ranks2[])

p. 218, line 12. Replace

#!c
int MPI_Group_incl(MPI_Group group, int n, int *ranks,
                   MPI_Group *newgroup)

by

#!c
int MPI_Group_incl(MPI_Group group, int n, int ranks[],
                   MPI_Group *newgroup)

p. 218, line 37. Replace

#!c
int MPI_Group_excl(MPI_Group group, int n, int *ranks,
                   MPI_Group *newgroup)

by

#!c
int MPI_Group_excl(MPI_Group group, int n, int ranks[],
                   MPI_Group *newgroup)

Chapter 7 (Process Topologies)

p. 248, line 37, C++ binding should be changed from

void MPI::Graphcomm::Get_dims(comm, int nnodes[], int nnedges[]) const

to

void MPI::Graphcomm::Get_dims(comm, int &nnodes, int &nnedges) const

p. 249, line 13-14: Replace

#!c
int MPI_Graph_get(MPI_Comm comm, int maxindex, int maxedges, int *index,
                  int *edges)

by

#!c
int MPI_Graph_get(MPI_Comm comm, int maxindex, int maxedges, 
                  int index[], int edges[])

p. 250, line 14-15: Replace

#!c
int MPI_Cart_get(MPI_Comm comm, int maxdims, int *dims, int *periods,
                 int *coords)

by

#!c
int MPI_Cart_get(MPI_Comm comm, int maxdims, 
                 int dims[], int periods[], int coords[])

p. 251, line 15: Replace

#!c
int MPI_Cart_coords(MPI_Comm comm, int rank, int maxdims, int *coords)

by

#!c
int MPI_Cart_coords(MPI_Comm comm, int rank, int maxdims, int coords[])

p. 251, line 46-47: Replace

#!c
int MPI_Graph_neighbors(MPI_Comm comm, int rank, int maxneighbors,
                        int *neighbors)

by

#!c
int MPI_Graph_neighbors(MPI_Comm comm, int rank, int maxneighbors,
                        int neighbors[])

Chapter 8 (Environmental Management)

p 313, line 8: Replace

#!c
int MPI_Init(int *argc, char ***argv)

by

#!c
int MPI_Init(int *argc, char *((*argv)[]))

Chapter 9 (Info)

No issues

Chapter 10 (Process Creation and Management)

What about character arrays?

#!c
int MPI_Open_port(MPI_Info info, char *port_name)

to

#!c
int MPI_Open_port(MPI_Info info, char port_name[])

and

#!c
void MPI::Open_port(const MPI::Info& info, char* port_name)

to

#!c
void MPI::Open_port(const MPI::Info& info, char port_name[])
#!c
int MPI_Lookup_name(char *service_name, MPI_Info info, char *port_name)

to

#!c
int MPI_Lookup_name(char *service_name, MPI_Info info, char port_name[])

and

#!c
void MPI::Lookup_name(const char* service_name, const MPI::Info& info, char* port_name)

to

#!c
void MPI::Lookup_name(const char* service_name, const MPI::Info& info, char port_name[])

Chapter 11 (One-sided)

No issues

Chapter 12 (External Interfaces)

No issues

Chapter 13 (I/O)

Chapter 14 (Profiling Interface)

No issues

Chapter 15 (Deprecated Functions)

Chapter 16 (Language Bindings)

No issues.

Impact on Implementations

Impact on Applications / Users

Alternative Solutions

Entry for the Change Log

@mpiforumbot
Copy link
Collaborator Author

Originally by bronis on 2009-02-10 19:02:03 -0600


Chapters 12 and 14 checked. No output array issues.

@mpiforumbot
Copy link
Collaborator Author

Originally by moody20 on 2009-02-13 17:23:56 -0600


Checked chapter 5 -- no issues found.

@mpiforumbot
Copy link
Collaborator Author

Originally by htor on 2009-02-28 21:45:32 -0600


double-checked chap 5 ;) - no issues

@mpiforumbot
Copy link
Collaborator Author

Originally by jsquyres on 2009-04-07 08:08:28 -0500


Fixed some wiki formatting.

@mpiforumbot
Copy link
Collaborator Author

Originally by jsquyres on 2009-04-07 08:10:40 -0500


No issues in language bindings chapter.

@mpiforumbot
Copy link
Collaborator Author

Originally by traff on 2009-04-07 10:38:12 -0500


Small inconsistency in C++ interface, p. 248, line 27, corrected (after consultation with Jeff)

@mpiforumbot
Copy link
Collaborator Author

Originally by rrtreu on 2009-04-07 14:34:39 -0500


The description seems ambiguous. The [] form applies to both input and output arrays. Why does this ticket make explicit statements about output arrays? Are input arrays already covered by a ticket that mostly deals with "const"?

If so, the ticket number that covers corrections for input arrays should be identified.

@mpiforumbot
Copy link
Collaborator Author

Originally by gropp on 2010-07-30 11:41:05 -0500


Noted that ticket #126 covers input arrays.

@mpiforumbot
Copy link
Collaborator Author

Originally by rlgraham on 2010-09-18 04:13:08 -0500


What do you want to do with this ticket ? rich

@mpiforumbot
Copy link
Collaborator Author

Originally by gropp on 2010-09-27 09:43:24 -0500


Each chapter committee should check on this - we should then, once each chapter committee verifies that they can make these changes, vote on the changes as a whole.

Note on the comment about input and output args - yes, these should be considered together with changes for arrays on input (see #126).

@mpiforumbot
Copy link
Collaborator Author

Originally by ftillier on 2012-01-20 10:28:51 -0600


Attachment added: ticket-125.pdf (2264.9 KiB)

@mpiforumbot
Copy link
Collaborator Author

Originally by ftillier on 2012-03-07 13:45:22 -0600


MPI Forum decided to make this "ticket 0" and it was passed with a single vote in the March 2012 Chicago (USA) meeting.

@mpiforumbot
Copy link
Collaborator Author

Originally by htor on 2012-07-03 22:33:40 -0500


Implemented in approved/topol.

Torsten

@mpiforumbot
Copy link
Collaborator Author

Originally by moody20 on 2012-07-12 19:09:30 -0500


TOPO CHAPTER REVIEW:

p307,40: MPI_Graph_create change correct, but mislabled as ticket125 instead of ticket126

needed on p305,40 for dims param?

otherwise, ok

@mpiforumbot
Copy link
Collaborator Author

Originally by moody20 on 2012-07-13 00:27:38 -0500


Rolf pointed out that I forgot to list which document my page and line numbers are for. I used this document provided by Torsten: http://www.unixer.de/sec/mpi-report-r1243.pdf

@mpiforumbot
Copy link
Collaborator Author

Originally by htor on 2012-07-13 16:40:07 -0500


Replying to moody20:

TOPO CHAPTER REVIEW:
Thanks Adam!

p307,40: MPI_Graph_create change correct, but mislabled as ticket125 instead of ticket126
Yes, I fixed that.

needed on p305,40 for dims param?
Yes, this must have been an oversight by the ticket author. I implemented it as ticket 125, please check!

otherwise, ok
Great!

Torsten

@mpiforumbot
Copy link
Collaborator Author

Originally by htor on 2012-07-13 17:11:52 -0500


Here is the new version (including the fixes) for other reviewers:

http://www.unixer.de/sec/mpi-report-r1279.pdf

Torsten

@mpiforumbot
Copy link
Collaborator Author

Originally by goodell on 2012-07-15 14:08:35 -0500


Topo chapter review:

In r1280, the proposed "MPI::Graphcomm::Get_dims" changes are not applied, but I think
this is correct because such a change would not be backwards compatible.
Otherwise OK.

@mpiforumbot
Copy link
Collaborator Author

Originally by moody20 on 2012-07-17 00:51:38 -0500


DT CHAP REVIEW (r1270.pdf):

p97,33 MPI_TYPE_INDEXED "int *array_of_blocklens" --> "int array_of_blocklens[]"

p97,34 MPI_TYPE_INDEXED "int *array_of_displacements" --> "int array_of_displacements[]"

@mpiforumbot
Copy link
Collaborator Author

Originally by moody20 on 2012-07-17 00:57:10 -0500


above applies to ticket 126 (input arrays) not this ticket (output arrays)

@mpiforumbot
Copy link
Collaborator Author

Originally by anhvo on 2012-07-17 11:08:23 -0500


Group, Contexts, Communicators, and Caching Chapter (r1301):
This ticket (ticket125) and ticket126 have not been applied to this chapter yet
p243 MPI_Group_translate_ranks
p246 MPI_Group_incl
p247 MPI_Group_excl

@mpiforumbot
Copy link
Collaborator Author

Originally by anhvo on 2012-07-17 13:33:18 -0500


EM chapter also missed the change on MPI_Init for this ticket. I've modified the description to indicate the necessary changes

@mpiforumbot
Copy link
Collaborator Author

Originally by gropp on 2012-07-17 13:36:39 -0500


Groups etc chapter updated. Thanks.

@mpiforumbot
Copy link
Collaborator Author

Originally by moody20 on 2012-07-17 16:10:52 -0500


Verified Torsten's updates to Topo chapter.

@mpiforumbot
Copy link
Collaborator Author

Originally by schulzm on 2012-07-18 12:24:26 -0500


From Bill, regarding the example in chapter 8 right below the change of **_argv to *((_argv)[]):
It should be consistent - _argv[], not *_argv . Do not mark the change in the example - leave it in verbatim mode and just make the change. This is necessary to run the code checker.

@mpiforumbot
Copy link
Collaborator Author

Originally by samcmill on 2012-07-18 17:02:08 -0500


Chapters 3, 6, and 10 still use !**argv in examples rather than *argv[].

@mpiforumbot
Copy link
Collaborator Author

Originally by gropp on 2012-07-18 17:16:07 -0500


Well spotted. Fixed in Chapter 6.

@mpiforumbot
Copy link
Collaborator Author

Originally by RolfRabenseifner on 2013-01-07 10:47:12 -0600


Since Sep. 21, 2012, this ticket is included in MPI-3.0 and the pdf is checked according
https://svn.mpi-forum.org/svn/mpi-forum-docs/trunk/meetings/2012-07-jul/mpi3-tickets.xlsx

Therefore, by proxy / on behalf of the ticket owner, I close it with priority "Ticket complete", resolution "Text committed".

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

1 participant