Skip to content

Conversation

@kawashima-fj
Copy link
Member

This PR consists of four commits and three of them are trivial. The third commit (b5c34c1) may require discussion. Please read my lengthy commit message. This issue was previously discussed in #1753 but I forgot it for a while so the chance to keep the compatibility between master and v2.x was lost. Sorry.

If this PR is pulled to master, I'll make a PR for v2.x but modify the commit to keep the binary compatibility of MPI_C_COMPLEX between v2.0.0 and v2.0.x.

@bosilca please review

Closes #1753

@bosilca
Copy link
Member

bosilca commented Aug 2, 2016

@kawashima-fj Down in the datatype engine I prefer to keep the full name of the type, ompi_mpi_c_float_complex instead of ompi_mpi_c_complex (to keep the consistency with the other similar types such as ompi_mpi_c_double_complex).

@kawashima-fj
Copy link
Member Author

@bosilca Do you mean MPI_C_COMPLEX in mpi.h and mpif.h should point ompi_mpi_c_float_complex (C) and its index (Fortran) instead of ompi_mpi_c_complex? In other words, make ompi_mpi_c_float_complex the canonical name and make ompi_mpi_c_complex the alias of it (or delete ompi_mpi_c_complex) in Open MPI implementation? If this understanding is correct, I'll update the PR.

@bosilca
Copy link
Member

bosilca commented Aug 2, 2016

@kawashima-fj this is indeed what I meant. thanks.

This commit add the following Fortran named constants which are
defined in the MPI standard but are missing in Open MPI.

- `MPI_LONG_LONG` (defined as a synonym of `MPI_LONG_LONG_INT`)
- `MPI_CXX_FLOAT_COMPLEX`
- `MPI_C_BOOL`

And this commit also changes the value of the following Fortran
named constant for consistency.

- `MPI_C_COMPLEX`
  `(MPI_C_FLOAT_COMPLEX` is defined as a synonym of this)

Each needs a different solution described below.

For `MPI_LONG_LONG`:

The value of `MPI_LONG_LONG` is defined to have a same value
as `MPI_LONG_LONG_INT` because of the following reasons.

1. It is defined as a synonym of `MPI_LONG_LONG_INT` in
   the MPI standard.
2. `MPI_LONG_LONG_INT` and `MPI_LONG_LONG` has a same value
   for C in `mpi.h`.
3. `ompi_mpi_long_long` is not defined in
   `ompi/datatype/ompi_datatype_module.c`.

For `MPI_CXX_FLOAT_COMPLEX`:

Existing `MPI_CXX_COMPLEX` is replaced with `MPI_CXX_FLOAT_COMPLEX`
bacause `MPI_CXX_FLOAT_COMPLEX` is the right name defined in MPI-3.1
and `MPI_CXX_COMPLEX` is not defined in MPI-3.1 (nor older).
But for compatibility, `MPI_CXX_COMPLEX` is treated as a synonym
of `MPI_CXX_FLOAT_COMPLEX` on Open MPI.

For `MPI_C_BOOL`:

`MPI_C_BOOL` is newly added. The value which `MPI_C_COMPLEX` had
used (68) is assinged for it because the value becomes no longer
in use (described later) and it is a suited position as a datatype
added on MPI-2.2.

For `MPI_C_COMPLEX`:

Existing `MPI_C_FLOAT_COMPLEX` is replaced with `MPI_C_COMPLEX`
and `MPI_C_FLOAT_COMPLEX` is changed to have the same value.
In other words, make `MPI_C_COMPLEX` the canonical name and
make `MPI_C_FLOAT_COMPLEX` an alias of it.
This is bacause the relation of these datatypes is same as
the relation of `MPI_LONG_LONG_INT` and `MPI_LONG_LONG`, and
`MPI_LONG_LONG_INT` and `MPI_LONG_LONG` are implemented like that.
But in the datatype engine, we use `ompi_mpi_c_float_complex`
instead of `ompi_mpi_c_complex` as a variable name to keep
the consistency with the other similar types such as
`ompi_mpi_c_double_complex` (see George's comment in open-mpi#1927).
We don't delete `ompi_mpi_c_complex` now because it is used in
some other places in Open MPI code. It may be cleand up in the future.

In addition, `MPI_CXX_COMPLEX`, which was defined only in the Open MPI
Fortran binding, is added to `mpi.h` for the C binding.

This commit breaks binary compatibility of Fortran `MPI_C_COMPLEX`.
When this commit is merged into v2.x branch, the change of
`MPI_C_COMPLEX` should be excluded.
The name of `MPI_INTEGER16` obtained using `MPI_TYPE_GET_NAME`
from Fortran program was incorrect (`MPI_INTEGER8` was obtained)
when `INTEGER*16` is not supported by a compiler.

This bug affects only the Fortran binding because `MPI_INTEGER16`
is not defined in `mpi.h` if a compiler does not support it.
@kawashima-fj kawashima-fj force-pushed the pr/fortran-named-constants branch from 1128b39 to 722e898 Compare August 2, 2016 13:44
@kawashima-fj
Copy link
Member Author

@bosilca I updated the PR. I didn't delete ompi_mpi_c_complex this time because it is used some other places especially ompi_datatype_basicDatatypes array.

@bosilca
Copy link
Member

bosilca commented Aug 2, 2016

👍

@kawashima-fj kawashima-fj merged commit 642acfb into open-mpi:master Aug 2, 2016
@kawashima-fj kawashima-fj deleted the pr/fortran-named-constants branch August 12, 2016 00:32
bosilca pushed a commit to bosilca/ompi that referenced this pull request Oct 3, 2016
This commit add the following Fortran named constants which are
defined in the MPI standard but are missing in Open MPI.

- `MPI_LONG_LONG` (defined as a synonym of `MPI_LONG_LONG_INT`)
- `MPI_CXX_FLOAT_COMPLEX`
- `MPI_C_BOOL`

And this commit also changes the value of the following Fortran
named constant for consistency.

- `MPI_C_COMPLEX`
  `(MPI_C_FLOAT_COMPLEX` is defined as a synonym of this)

Each needs a different solution described below.

For `MPI_LONG_LONG`:

The value of `MPI_LONG_LONG` is defined to have a same value
as `MPI_LONG_LONG_INT` because of the following reasons.

1. It is defined as a synonym of `MPI_LONG_LONG_INT` in
   the MPI standard.
2. `MPI_LONG_LONG_INT` and `MPI_LONG_LONG` has a same value
   for C in `mpi.h`.
3. `ompi_mpi_long_long` is not defined in
   `ompi/datatype/ompi_datatype_module.c`.

For `MPI_CXX_FLOAT_COMPLEX`:

Existing `MPI_CXX_COMPLEX` is replaced with `MPI_CXX_FLOAT_COMPLEX`
bacause `MPI_CXX_FLOAT_COMPLEX` is the right name defined in MPI-3.1
and `MPI_CXX_COMPLEX` is not defined in MPI-3.1 (nor older).
But for compatibility, `MPI_CXX_COMPLEX` is treated as a synonym
of `MPI_CXX_FLOAT_COMPLEX` on Open MPI.

For `MPI_C_BOOL`:

`MPI_C_BOOL` is newly added. The value which `MPI_C_COMPLEX` had
used (68) is assinged for it because the value becomes no longer
in use (described later) and it is a suited position as a datatype
added on MPI-2.2.

For `MPI_C_COMPLEX`:

Existing `MPI_C_FLOAT_COMPLEX` is replaced with `MPI_C_COMPLEX`
and `MPI_C_FLOAT_COMPLEX` is changed to have the same value.
In other words, make `MPI_C_COMPLEX` the canonical name and
make `MPI_C_FLOAT_COMPLEX` an alias of it.
This is bacause the relation of these datatypes is same as
the relation of `MPI_LONG_LONG_INT` and `MPI_LONG_LONG`, and
`MPI_LONG_LONG_INT` and `MPI_LONG_LONG` are implemented like that.
But in the datatype engine, we use `ompi_mpi_c_float_complex`
instead of `ompi_mpi_c_complex` as a variable name to keep
the consistency with the other similar types such as
`ompi_mpi_c_double_complex` (see George's comment in open-mpi#1927).
We don't delete `ompi_mpi_c_complex` now because it is used in
some other places in Open MPI code. It may be cleand up in the future.

In addition, `MPI_CXX_COMPLEX`, which was defined only in the Open MPI
Fortran binding, is added to `mpi.h` for the C binding.

This commit breaks binary compatibility of Fortran `MPI_C_COMPLEX`.
When this commit is merged into v2.x branch, the change of
`MPI_C_COMPLEX` should be excluded.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants