[MPI3 Fortran] MPI draft 2011-04-21

[Rolf Rabenseifner]

Hi Craig,

thank you for your detailed review.
Answers and comments are included in the text below.
For some important issues I have questions to you, see below.

Best regards
Rolf

----- Original Message -----
> From: "Craig E Rasmussen" <rasmussn at lanl.gov>
> To: "Rolf Rabenseifner" <rabenseifner at hlrs.de>
> Cc: "MPI-3 Fortran working group" <mpi3-fortran at lists.mpi-forum.org>
> Sent: Monday, May 2, 2011 8:51:46 PM
> Subject: Re: [MPI3 Fortran] MPI draft 2011-04-21
> Rolf,
> 
> I've finished with proposal writing madness and I've now had time to
> review the MPI draft 2011-04-21. Please find detailed comments below:
> 
> -craig
> 
> 
> 
> --------------------------------
> 
> Pg 543 2nd paragraph: There is a discussion of MPI_SUBARRAYS_SUPPORTED
> here. I think we should also add an advice to implementers regarding
> MPI_SUBARRAYS_SUPPORTED. This advice should include information about
> the need to copy possibly non-contiguous regions of memory specified
> by an array descriptor.

The advice to implementors 
 Draft 2011-04-21 p546:5-9
 Draft 2011-05-02 p548:5-9
tries to give this information. It is enough? Do you want any changes?
 
> Pg 543, line 26: insert "may" ==> "While these may cause few problems"
> because the original MPI Fortran bindings violate the Fortran standard
> and compilers are becoming more aggressive at catching these
> violations.

Yes, I'll change it.

> Pg 543, line 31: The list is also inconsistent with implicit
> interfaces ("Fortran 77" style) as well as explicit interfaces. This
> discussion gives the wrong impression that Fortran 90 is inferior to
> Fortran 77.

Yes, I'll add "and Fortran 77" on line 31.

> Pg 543, line 45: These named "constants" are an implementation detail
> and I don't understand why this is more than advice to implementors.

It is important, that the user must not copy these constants.
  buf = MPI_BOTTOM
  MPI_SEND(buf, ...)
is invalid. Therefore here the reason and a pointer to Section 2.5.4.
 
> Pg 544, line 4: MPI_ALLOC_MEM should be defined so that the language
> interoperability features in F2003 can be used to associate the
> allocated memory with a Fortran variable.
 
This is still on my TODO list for the rest of this week.
It is not yet solved in Draft 2011-05-02. Sorry for my delay.

> Pg 544, line 36: With combinations of choice arguments it is
> impractical to provide overloaded functions for all argument
> combinations.

Yes, I like to agree. The sentence here was not intended to say 
that this would be a practical solution. More detailed discussion
is on
 Draft 2011-04-21 p561:4-5
 Draft 2011-05-02 p563:30-31
with a citation of Hennecke's paper.
Do you still think, I should modify it?

> Pg 544, line 40: Replace sentence fragment with: "the following code
> fragment is technically invalid and may generate ..."

Yes, I agree and change it back from "might be" to the original "is".

> Pg 545: line 6 and 11: DIMENSION(..) in mpi_f08 allows scalar
> arguments for choice buffers. Replace "USE mpi_f08" with "USE mpi"

After "to an array dummy argument", I'll add on 
 Draft 2011-04-21 p545:6
 Draft 2011-05-02 p547:6
"that is not a choice buffer argument"

Within the example, I'll substitute
  USE mpi_f08
by 
  USE mpi_f08  ! (or USE mpi)

Reasons: 
 - In current mpi module implementations, these arguments must be arrays. 
 - There is no need and no reason to change it within the mpi_f08 to
    INTEGER, DIMENSION(..) dims 
   It would only make the MPI library implementation more complicated
   and therefore probably with higher latency.
 
> Pg 545, line 15: Replace original phrase with "compiler warnings are
> not expected (but may occur) unless ..."

Yes, I'll change it.

> Pg 545, line 19: Delete "in" in the phrase "describe in Fortran
> subarrays"

Yes, I'll change it.

> Pg 545, line 36: Replace "compiled code must not" with "compiled code
> will not"

Yes, I'll change it.

> Pg 545, line 29: Delete ", ..., s(96)" as there are only 3 elements
> send in the MPI_Isend call.

The descriptor (made by the Fortran compiler) allows to operate
on s(1), s(6), ..., s(96). The MPI_Send routine uses only
the first three of these elements due to the arguments "3, MPI_Real".

Therefore, I would say that the existing text is correct.
I'll add the sentence:

  The called MPI_ISEND routine will take only the first three
  of these elements due to the type signature "3, MPI_REAL". 

> Pg 545, line 41: I would put the phase "as if" in italics

Yes, I'll change it.

> Pg 546, lines 16 and 20: The "_NOT_" in MPI_SUBARRAYS_NOT_SUPPORTED is
> in a extra large font.

Yes, I used the larger font with intent to highlight the "NOT".
Bold font in combination with sans-serif does not work good enough. 

> Pg 546, line 29: The discussion related to the phrase "Most compilers
> ensure ..." is true (I think, although I haven't seen a compiler do
> otherwise) but I believe if the interfaces are defined with the
> BIND(C) attribute that the compiler will be forced to make a copy.

More and more you convince me that BIND(C) interfaces should not
be used on the level of the MPI library interface.
It is fine for the C routine called within a Fortran written
wrapper, as done in your implementation.

> Pg 546, footnote 1: Replace "Fortran standards are worded" with
> "Fortran standard is worded"

Yes, I'll change it.

> Pg 547, line 24: Replace "Some compilers make a copy of some scalar"
> with "A compiler may make a copy of scalar"

Yes, I'll change it.

> Pg 547, line 28: Insert "real :: a" in the example to make the scalar
> argument clear

Yes, I'll change it.

> Pg 548, line 14: Replace "an possibly" with "a possibly"

Yes, I'll change it.

> Pg 548, paragraph starting at line 25: I don't think the discussion
> regarding constants/parameters is true as because an implementor can
> directly link to C variables with BIND(C), e.g.,
>
> type(MPI_Group), protected, bind(C, name="ompi_f08_mpi_group_null") ::
> MPI_GROUP_NULL

This discussion is about MPI_BOTTOM, MPI_IN_PLACE, MPI_ARGV_NULL
and MPI_ARGVS_NULL in mpi_f08 and also four others in the mpi module.

It is not about the "normal constants, like MPI_COMM_NULL, MPI_STATUS_SIZE, ...

In Draft 2011-05-02 p550:33-34, I changed the last "black" sentence 
into a new "red":
 
  Inside the subroutine, the address of the actual choice
  buffer argument can be compared with the address of such 
  a predefined static variable.
 
> Pg 548, line 43 and example on page 549: I think "sequence" should be
> "SEQUENCE" to be consistent. 

Yes, I'll change it.

> Pg 548, line 43 and example on page 549: 
> However I don't agree with the entire
> argument about sequence types. The example on page 549 uses
> displacements based on calls to MPI_GET_ADDRESS. I'm probably missing
> something because I think this should work irrespective of the use of
> SEQUENCE because the MPI data type is constructed with calls to
> MPI_GET_ADDRESS. However, the type will not be interoperable across C
> and Fortran because the struct is not declared with BIND(C).

Does the Fortran standard guarantee that an array of a derived type
or a derived type with arrays as structure elements is internally
always stored as defined:

  TYPE :: struct
    REAL :: x, y, z
    INTEGER :: geo
    END TYPE struct
  TYPE(struct) a(1000000)
  DO i=1,1000000
   a(i)%x = fx(i)
  END DO
  DO i=1,1000000
   a(i)%y = fy(i)
  END DO
  DO i=1,1000000
   a(i)%z = fz(i)
  END DO
  CALL MPI_SEND(a(1)%x, ....)

Is the fortran compiler allowed to store it
as 

  TYPE :: struct_iternal
    REAL :: x(1000000), y(1000000), z(1000000)
    INTEGER :: geo(1000000)
    END TYPE struct
  TYPE(struct) a

I was told, that without the SEQUENCE attribute,
the compiler can do what it expect to be most efficient.
Is this true?  

 
> Pg 550 (and elsewhere): I would use "Code movement" rather than "Code
> movements" and "Data movement" rather than "Data movements",
> especially as data is plural anyway.

Yes, I'll change it.

> Pg 550, line 37: Replace "or COMMON block" with "or a COMMON block"

Yes, I'll change it.

> Pg 551, line 17: Don't understand where "send(*addr)" comes from.
> Shouldn't it be MPI_Isend or MPI_Wait or something similar?

I'll add the comment "within MPI_WAIT":

    send(*addr) !within MPI_WAIT

> Pg 551, line 27: I would insert "by the compiler" at the end of the
> sentence.

Yes, but I'll add it after "overwritten": 

   ... overwritten by the compiler when the content of buf is sent.

> Pg 551, line 31: I don't understand why MPI_BOTTOM and derived MPI
> datatypes are special, we should talk about this at the next forum
> meeting.

Jeff asked me to add a sentence to make clear, that the split
collective method with MPI_..._begin(buf,...) and MPI_..._end(buf,...)
helps only for normal buffers, but not for MPI_BOTTOM.

With the application
  buf = a
  MPI_...write..._begin(MPI_BOTTOM,1,dt,...) 
    ! and "MPI_BOTTOM,1,dt" describing "buf"
  MPI_...write..._end(MPI_BOTTOM,1,dt, ....)
  buf = b

the compiler is still allowed to modify into

  MPI_...write..._begin(MPI_BOTTOM,1,dt,...) 
  MPI_...write..._end(MPI_BOTTOM,1,dt, ....)
  ! buf = a  ! removed dead code
  buf = b

  
> Pg 552, two examples: These examples remind me that we need to
> carefully think where the TARGET attribute should/must be used in a
> valid MPI program.

The other solutions for examples 16.13 and 16.14 
have been there and were described since July 1997.
I hope that they are still correct and that there is no case
where TARGET ***must be used*** instead of the other solutions.
 
> Pg 552, line 43: Replace this line with "operations in which
> MPI_BOTTOM is used or datatype handles that combine several variables
> are used:"

Yes, I'll change it.

> Pg 553, line 4: Replace "can solve" with "may solve"

I remove the "can", i.e. write only "solves".
See next item:

> Pg 553, line 6: Insert something like "MPI_F_SYNC_REG and the use of
> module data" are not guaranteed to work by the Fortran standard,
> though will likely work on most compilers.

I'll add after "Each of these methods solve ... every application context.":

  These methods may not be guaranteed by the Fortran standard,
  but they must be guaranteed by a \MPIIIIDOTO/ compliant (and later)
  \MPI/ library and their compiler according to the requirements 
  listed in Section 16.2.16 on page 566.
   
> Pg 554, first paragraph: MPI_F_SYNC_REG should be made BIND(C) to keep
> Fortran from very aggressive optimizations.

I have appended new text in Draft 2011-05-02 p568:25-36.
I keep this as an advice to implementors.

> Pg 555, line 16+: Remove discussion of DD. Compiler more likely to
> optimize user function that something MPI vendor writes and tests.

It should be kept for historical reasons.
I've added new text in Draft 2011-05-02 p557:46-48:

  Such a user-defined routine was introduced in MPI-2.0 
  and is still included here to document such usage in 
  existing application programs although new applications 
  should prefer MPI_F_SYNC_REG or one of the other possibilities.

I agree with your risk-analysis. Therefore I'll add:

  In existing application, calls to such a user-written routine should 
  be substituted by a call to MPI_F_SYNC_REG because the
  user-written routine may not be implemented according to the
  rules specified in Section 16.2.16 on page 566.

See also below the discussion of DD. 
 
> Pg 556, line 1: Replace "(evil)" with :(poorly performing)"

You are the first one who wants to substitute "evil".
I'll change it ;-)
 
> Pg 556: I would move Fortran language attribute (VOLATILE and
> ASYNCHRONOUS) usage discussion above what are essentially hacks that
> are outside of the language (MPI_F_SYNC_REG and Module Data).

I sorted it more based on usability and performance arguments.
More user's view than the view of the Fortran (or MPI) standardization 
bodies.

> Pg 556, line 14: I would add a discussion of the TARGET attribute and
> its usage

Draft 2011-05-02 p555:46 - p556:13 is the discussion 
within Section 16.2.9.  

I agree that some words are also needed within Section 16.2.10.

I'll add after Draft 2011-05-02 p560:23:

  Note also that the methods 
   - TARGET attribute, 
   - calling MPI_F_SYNC_REG (or such a user-defined routine), and 
   - using module variables and COMMON blocks 
  cannot be used to prevent such temporary data movement.
  These methods influence compiler optimization when
  library routines are called. They cannot prevent the optimizations
  of the numerical code shown in Example 16.15 and 16.16.

> Pg 556, line 17: Replace "modify temporarily" with "temporarily
> modify"

Yes, I'll change it.

> Pg 556, line 44: Replace "may also occurs" with "may also occur"

Yes, I'll change it.

> Pg 557, line 19: Replace "of a local memory" with "of a separate
> memory storage area"

Yes, I'll change it.

> Pg 557, line 34: Should "the window buffer" be "the MPI window
> buffer"? I not sure what window references. Also, is RMA correct?

"window buffer" is okay. 
"RMA call" must be changed into "RAM communication call".
And I add references to Chapter 11 and Section 13.4.5.

> Pg 557, line 45: "Said this" should probably be replaced with "This
> said" or "Thus"

Yes, I'll change it.

> Pg 558, lines 1-3: ASYNCHRONOUS may actually be fixed at WG5 so lines
> 2-3 may in fact turn out to be an incorrect statement.

Let's look what WG5 is doing.

> Pg 558, lines 8-18: ASYNCHRONOUS is used here where the discussion is
> about VOLATILE

I've a problem, because your page-lines do not fit to the version 
on the ticket.

I expect that you discuss a conflict of 
 - usage of ASYNCHRONOUS in Example 16.17
 - and the paragraph about VOLATILE.

The figure is located by luck somewhere. The figure is discussed
in some previous section. 

The last part of this Section is organized like this:
  - first a note about non-usable ASYNCHRONOUS,
  - second the new note non-usable TARGET, MPI_F_SYNC_REG, and
    module variables,
  - third a note about inefficient VOLATILE
  - and last the advice to prevent the problem.

> Pg 559, lines 47-48: Are there really any "Fortran 77" compilers
> around? The Fortran standard accepts ! as starting a comment for both
> fixed- and free-form formatted files.

I do not know. There is no need to remove it because it should be still correct.
 
> Pg 563, line 6: should be reworded with something like "BIND(C) allows
> vendors to implement the ..."

I would prefer to keep the passive, because we use the word
"implementors" to summarize vendors and public domain implementors.

I do not want to write "allows implementors to implement"
  
> Pg 563, line 41: I would replace "memorize" with "store"

Yes, I'll change it.

> Pg 563, last paragraph: This is not guaranteed by the Fortran
> standard.

Yes, it is not guaranteed by the Fortran standard.
This section is mainly about guarantees that are
additionally necessary to be MPI-3.0 compliant.

Discussion about DD, this paragraph reads:

  A separately compiled empty routine (as MPI_F_SYNC_REG on 
  page 556 and Section 16.2.17 on page 568, and DD on page 557) 
  solves the problems described in Section 16.2.9 on page 552.

The MPI Forum is in the conflict that it should keep the MPI
standard backward compatible. Thus, the DD must stay a valid
solution.
MPI-2.0 - MPI-2.2 uses the wording "external subroutine"
and "separately compiled" and it has some wording about
INTENT "if intent is declared".

A compromise would be to require that a separately compiled 
Fortran routine with implicit interface or a BIND(C) interface
with separately compiled C routine is guaranteed to work.
The problem would be if an application has used a
module that CONTAINS such an empty routine and the compiler
has stored knowledge about the contained routine in
the .mod module information.

Therefore, I would propose to change the text into:

  Separately compiled empty Fortran routines with implicit interfaces
  and separately compiled empty C routines with BIND(C) Fortran interfaces
  (as MPI_F_SYNC_REG on page 556 and Section 16.2.17 on page 568, 
  and DD on page 557) solve the problems described in 
  Section 16.2.9 on page 552.

Okay?

> Pg 564, 4th bullet: I disagree with the 4th bullet as I believe it
> ultimately breaks the Fortran standard. How can two different types
> (INTEGER and a derived-type handle) both be valid to the same
> interface?

The user can write the following codes without any problem:

  SUBROUTINE foo1(oldcomm, oldsts)
  USE mpi_f08
  INTEGER :: oldcomm, oldsts(MPI_STATUS_SIZE)
  INTEGER :: ierror
  TYPE(MPI_Status) :: newsts
  TYPE(MPI_Comm) :: newcomm
  newcomm%MPI_VAL = oldcomm
  CALL MPI_RECV(....newcomm, newsts, ierror)
  CALL MPI_Status_f082f(newsts, oldsts, ierror)
  END

  SUBROUTINE foo2(newcomm, newsts)
  USE mpi
  TYPE(MPI_Comm)   :: newcomm
  TYPE(MPI_Status) :: newsts
  INTEGER :: ierror
  INTEGER :: oldcomm, oldsts(MPI_STATUS_SIZE)
  oldcomm = newcomm%MPI_VAL
  CALL MPI_RECV(....oldcomm, oldsts, ierror)
  CALL MPI_Status_f2f08(oldsts, newsts, ierror)  
  END

I do not see any conflict.
In each module, there is only one well-defined interface 
for each routine and constant.

This paragraph should tell:

  SUBROUTINE foo1
    USE mpi
    INTEGER comm
    COMMON /mycomm/ comm
    CALL MPI_SEND( ...., comm,...)
  END

should be convertable into

  SUBROUTINE foo1
    USE mpi_f08          ! line changed
    TYPE(MPI_Comm) comm  ! line changed
    COMMON /mycomm/ comm
    CALL MPI_SEND( ...., comm,...)
  END

with only these two changes.
  
Another application

  MODULE mydata
    USE mpi
    TYPE :: xyz
      SEQUENCE
      INTEGER :: comm
      ...
    END TYPE xyz
    TYPE(xyz) :: set1
  END MODULE mydata

  SUBROUTINE foo2
    USE mpi
    USE mydata
    CALL MPI_SEND( ...., set1%comm,...)
  END

should be convertable into

  MODULE mydata
    USE mpi_f08            ! line changed
    TYPE :: xyz
      SEQUENCE
      TYPE(MPI_Comm) comm  ! line changed
      ...
    END TYPE xyz
    TYPE(xyz) :: set1
  END MODULE mydata

  SUBROUTINE foo2
    USE mpi_f08          ! line changed
    USE mydata
    CALL MPI_SEND( ...., set1%comm,...)
  END

Do we also want to allow that

  PROGRAM foo4
    USE mpi
    TYPE, BIND(C) :: mydata
      INTEGER :: comm
       ...
    END TYPE mydata
    TYPE(mydata) :: mystruct
    CALL MPI_SEND( ...., mystruct%comm,...)
  END

should be convertable into

  PROGRAM foo4
    USE mpi_f08            ! line changed
    TYPE, BIND(C) :: mydata
      TYPE(MPI_Comm) comm  ! line changed
       ...
    END TYPE mydata
    TYPE(mydata) :: mystruct
    CALL MPI_SEND( ...., mystruct%comm,...)
  END

Currently, this example currently works only with ifort and PGI.
It looks like, that it is hard to get it into the Fortran standard,
but it looks like that it is easy to implement as long as
int and INTEGER are all 4 bytes aligned on 4 byte boundaries.

Therefore, putting it into the requirement section is okay
although the Fortran committee never allows this as part
of TR29113.

In a world with 8 byte INTEGER and 4 byte int, it will be more
complicated to achieve full MPI-3.0 compliance.


> Pg 564, lines 34-38: Disagree with any discussion of additional
> compiler support needed beyond TR 29113. If this is related to
> SEQUENCE and BIND(C) I don't think it will ever happen (I don't think
> it will get a single yes vote from the Fortran committee) and would
> not be in the standard for 3-5 years anyway. See also SEQUENCE
> discussion in last paragraph on this page.

The text (Draft 2011-05-02 p567:32-33)

  "Some of these requirements for MPI-3.0 are beyond of TR 29113."

tries to summarize:
 - Some optimizations may need to be switched off within 
   the mpif90 scripts.
 - Some compiler need to switch some "error" messages into
   "warnings"
 - Some compilers are not MPI-3.0 compliant as long as they
   give not the needed support for these requirements.

It is important for the user to know that his/her application
will be compiled and executed correctly.
It is also important that he/she can use every MPI-3.0
library as long as his/her code is portable according to
the coding rules defined in MPI-3.0.


> Pg 564, line 41: I suggest that CHARACTER*(*) be replaced with more
> modern usage.

Yes, I'll change it.

> Pg 565: Regarding MPI_F_SYNC_REG: In advice to implementors this
> function should be made BIND(C) to, perhaps, disallow aggressive
> optimizations.

You send a long list, sorry that my answer cannot be shorter.
I hope that my answers help on the path to our final documents.

Best regards
Rolf

-- 
Dr. Rolf Rabenseifner . . . . . . . . . .. email rabenseifner at hlrs.de
High Performance Computing Center (HLRS) . phone ++49(0)711/685-65530
University of Stuttgart . . . . . . . . .. fax ++49(0)711 / 685-65832
Head of Dpmt Parallel Computing . . . www.hlrs.de/people/rabenseifner
Nobelstr. 19, D-70550 Stuttgart, Germany . (Office: Allmandring 30)