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

libnbc nonuniform type failures in ibcast, iallgather(v) #2256

Closed
jjhursey opened this issue Oct 20, 2016 · 10 comments
Closed

libnbc nonuniform type failures in ibcast, iallgather(v) #2256

jjhursey opened this issue Oct 20, 2016 · 10 comments

Comments

@jjhursey
Copy link
Member

When testing coll/libnbc with nonuniform data types three of the algorithms failed. I'll post a link to the test case in the comments. The unit test only works with -np 5 some of the tests pass fine with -np 4.

  • ibcast Falls into an infinite error loop in libnbc
  • iallgather and iallgatherv produce wrong answers. Might be the same underlying problem.

ibcast Failure

Note: This test will enter an infinite loop displaying MPI Error in MPI_Testall() (18) until PR #2245 is resolved.

shell$ mpirun -np 5 --mca coll ^hcoll  ./test-nbc-dt 0 
 0 /  5) Running MPI_Ibcast...
 1 /  5) Running MPI_Ibcast...
 2 /  5) Running MPI_Ibcast...
 3 /  5) Running MPI_Ibcast...
 4 /  5) Running MPI_Ibcast...
MPI Error in MPI_Testall() (18)
MPI Error in MPI_Testall() (18)
...

iallgather Failure

shell$ mpirun -np 5 --mca coll ^hcoll  ./test-nbc-dt 1
 0 /  5) Running MPI_Iallgather...
 1 /  5) Running MPI_Iallgather...
 2 /  5) Running MPI_Iallgather...
 3 /  5) Running MPI_Iallgather...
 4 /  5) Running MPI_Iallgather...
 3 /  5) buf[1] : actual 2 [2], expected 1 [1]. from 0
 3 /  5) Failed in chkbuf. err = -1
--------------------------------------------------------------------------
MPI_ABORT was invoked on rank 3 in communicator MPI_COMM_WORLD
with errorcode -2.

NOTE: invoking MPI_ABORT causes Open MPI to kill all MPI processes.
You may or may not see output from other processes, depending on
exactly when Open MPI kills them.
--------------------------------------------------------------------------
 2 /  5) buf[1] : actual 2 [2], expected 1 [1]. from 0
 2 /  5) Failed in chkbuf. err = -6
 1 /  5) buf[2] : actual 2 [2], expected 1 [1]. from 0
 1 /  5) Failed in chkbuf. err = -2
 4 /  5) buf[1] : actual 2 [2], expected 1 [1]. from 0
 4 /  5) Failed in chkbuf. err = -6

iallgatherv Failure

shell$ mpirun -np 5 --mca coll ^hcoll  ./test-nbc-dt 2
 0 /  5) Running MPI_Iallgatherv...
 1 /  5) Running MPI_Iallgatherv...
 2 /  5) Running MPI_Iallgatherv...
 3 /  5) Running MPI_Iallgatherv...
 4 /  5) Running MPI_Iallgatherv...
 0 /  5) buf[1] : actual 3 [3], expected 2 [2]. from 1
 0 /  5) Failed in chkbuf. err = -6
 3 /  5) buf[1] : actual 3 [3], expected 2 [2]. from 1
 3 /  5) Failed in chkbuf. err = -6
 4 /  5) buf[1] : actual 3 [3], expected 2 [2]. from 1
 4 /  5) Failed in chkbuf. err = -6
--------------------------------------------------------------------------
MPI_ABORT was invoked on rank 3 in communicator MPI_COMM_WORLD
with errorcode -2.

NOTE: invoking MPI_ABORT causes Open MPI to kill all MPI processes.
You may or may not see output from other processes, depending on
exactly when Open MPI kills them.
--------------------------------------------------------------------------
 2 /  5) buf[1] : actual 3 [3], expected 2 [2]. from 1
 2 /  5) Failed in chkbuf. err = -6
@jjhursey jjhursey added this to the v2.0.2 milestone Oct 20, 2016
@jjhursey
Copy link
Member Author

Here is the test case for this issue:
https://gist.github.com/jjhursey/dfbb38c7c82dcde7d0e05e62852fce3c

Sorry it's not more general, but should help reproduce the problem.

ggouaillardet added a commit that referenced this issue Oct 21, 2016
In order to optimize for MPI_IN_PLACE, data is sent from the receive buffer.
consequently, it should be sent with the receive type and count.

Thanks Josh Hursey for the report and test case

Refs #2256
@ggouaillardet
Copy link
Contributor

@jjhursey i fixed MPI_Iallgather[v] in #2245

the issue with MPI_Ibcast is the very same and unresolved issue we have in coll/tuned.
we should not try to split a message into chunks since we cannot be sure the remote peer is able to split the message into chunks of the same size.
for example, if you need to MPI_Send 1M MPI_INT, it is trivial to MPI_Send 1k MPI_INT 1024 times (and this is what we do in ibcast and coll/tuned
but if the remote peer MPI_Recv one vector of 1M MPI_INT, there is currently no way to split that into 1024 MPI_Recv of partial vector.
we currently assume we do not run into this case and the remote peer will be just fine, and once in a while, we are reminded the assumption is wrong.
fwiw, i started to work on #1789 as a proof of concept, but had not much time to finalize it

ggouaillardet added a commit to ggouaillardet/ompi that referenced this issue Oct 21, 2016
In order to optimize for MPI_IN_PLACE, data is sent from the receive buffer.
consequently, it should be sent with the receive type and count.

Thanks Josh Hursey for the report and test case

Refs open-mpi#2256

(cherry picked from commit open-mpi/ompi@45336d0)
ggouaillardet added a commit to ggouaillardet/ompi that referenced this issue Oct 21, 2016
In order to optimize for MPI_IN_PLACE, data is sent from the receive buffer.
consequently, it should be sent with the receive type and count.

Thanks Josh Hursey for the report and test case

Refs open-mpi#2256

(cherry picked from commit open-mpi/ompi@45336d0)
ggouaillardet added a commit to ggouaillardet/ompi that referenced this issue Oct 21, 2016
In order to optimize for MPI_IN_PLACE, data is sent from the receive buffer.
consequently, it should be sent with the receive type and count.

Thanks Josh Hursey for the report and test case

Refs open-mpi#2256

(back-ported from commit open-mpi/ompi@45336d0)
@jjhursey
Copy link
Member Author

@ggouaillardet I can confirm that your commit fixed the problem with the MPI_Iallgather and MPI_Iallgatherv collectives. Thanks!

Adding a reference to Issue #1763 - which describes the data corruption problem with MPI_Bcast. Since those two are related problems, a fix for one should be ported to the other.

As a workaround I might look into adding an MCA parameter to skip the algorithm selection based upon message size for bcast.

@ggouaillardet
Copy link
Contributor

@jjhursey IIRC, coll/tuned has a (per collective ?) segment_size MCA param, and we recommend it is set to 0 when running into this kind of pattern. You might want to use the same naming scheme for libnbc.
An other workaround is to blacklist coll/tuned, but that cannot be mimicked for non-blocking collectives.

@ggouaillardet
Copy link
Contributor

Btw, if coll/tuned are used, the default behavior is MPI_ERR_TRUNCATE causes the program abort.
If coll/libnbc is used instead, should we follow the same logic (e.g. call the error handler, which aborts by default) ?

jjhursey added a commit to jjhursey/ompi that referenced this issue Nov 1, 2016
 * If (legal) non-uniform data type signatures are used in ibcast
   then the chosen algorithm may fail on the request, and worst case
   it could produce wrong answers.
 * Add an MCA parameter that, by default, protects the user from this
   scenario. If the user really wants to use it then they have to
   'opt-in' by setting the following parameter to false:
   - `-mca coll_libnbc_ibcast_skip_dt_decision f`
 * Once the following Issues are resolved then this parameter can
   be removed.
   - open-mpi#2256
   - open-mpi#1763
 * Fixes STG Defect #114680

Signed-off-by: Joshua Hursey <jhursey@us.ibm.com>
jjhursey added a commit to jjhursey/ompi that referenced this issue Nov 1, 2016
 * If (legal) non-uniform data type signatures are used in ibcast
   then the chosen algorithm may fail on the request, and worst case
   it could produce wrong answers.
 * Add an MCA parameter that, by default, protects the user from this
   scenario. If the user really wants to use it then they have to
   'opt-in' by setting the following parameter to false:
   - `-mca coll_libnbc_ibcast_skip_dt_decision f`
 * Once the following Issues are resolved then this parameter can
   be removed.
   - open-mpi#2256
   - open-mpi#1763

Signed-off-by: Joshua Hursey <jhursey@us.ibm.com>
@jjhursey
Copy link
Member Author

jjhursey commented Nov 1, 2016

I created PR #2330 which adds a new MCA parameter to avoid this datatype size based decision. It's not ideal (since the NBC_BCAST_CHAIN algorithm will not be selected), but should work around the problem until it can be fixed.

Let me know if you have any ideas for improvements on that PR.

@hppritcha
Copy link
Member

maybe this should be moved to 2.1 for a more complete solution?

@jjhursey
Copy link
Member Author

jjhursey commented Nov 7, 2016

Yeah. I'll PR over my bcast parameter to the 2.(0.)x series. Then we can move it to v2.1 for a complete bcast solution.

jjhursey added a commit to jjhursey/ompi that referenced this issue Nov 7, 2016
 * If (legal) non-uniform data type signatures are used in ibcast
   then the chosen algorithm may fail on the request, and worst case
   it could produce wrong answers.
 * Add an MCA parameter that, by default, protects the user from this
   scenario. If the user really wants to use it then they have to
   'opt-in' by setting the following parameter to false:
   - `-mca coll_libnbc_ibcast_skip_dt_decision f`
 * Once the following Issues are resolved then this parameter can
   be removed.
   - open-mpi#2256
   - open-mpi#1763

Signed-off-by: Joshua Hursey <jhursey@us.ibm.com>
(cherry picked from commit 350ef67)
Signed-off-by: Joshua Hursey <jhursey@us.ibm.com>
jjhursey added a commit to jjhursey/ompi that referenced this issue Nov 7, 2016
 * If (legal) non-uniform data type signatures are used in ibcast
   then the chosen algorithm may fail on the request, and worst case
   it could produce wrong answers.
 * Add an MCA parameter that, by default, protects the user from this
   scenario. If the user really wants to use it then they have to
   'opt-in' by setting the following parameter to false:
   - `-mca coll_libnbc_ibcast_skip_dt_decision f`
 * Once the following Issues are resolved then this parameter can
   be removed.
   - open-mpi#2256
   - open-mpi#1763

Signed-off-by: Joshua Hursey <jhursey@us.ibm.com>
(cherry picked from commit 350ef67)
Signed-off-by: Joshua Hursey <jhursey@us.ibm.com>
@jjhursey
Copy link
Member Author

jjhursey commented Nov 8, 2016

We talked about this on the call today. Once the two work around PRs are merged (#2378, #2379). Then we will close this issue as fixed.

Once that happens then we should followup with a fix on Issue #1763 (I've added a note there with a reference)

@jjhursey
Copy link
Member Author

jjhursey commented Nov 9, 2016

I'm closing this issue now that the work around has been put into the v2.(0.).x branches. Followup work will be on Issue #1763

@jjhursey jjhursey closed this as completed Nov 9, 2016
artpol84 pushed a commit to artpol84/ompi that referenced this issue Dec 20, 2016
 * If (legal) non-uniform data type signatures are used in ibcast
   then the chosen algorithm may fail on the request, and worst case
   it could produce wrong answers.
 * Add an MCA parameter that, by default, protects the user from this
   scenario. If the user really wants to use it then they have to
   'opt-in' by setting the following parameter to false:
   - `-mca coll_libnbc_ibcast_skip_dt_decision f`
 * Once the following Issues are resolved then this parameter can
   be removed.
   - open-mpi#2256
   - open-mpi#1763

Signed-off-by: Joshua Hursey <jhursey@us.ibm.com>
(cherry picked from commit 350ef67)
Signed-off-by: Joshua Hursey <jhursey@us.ibm.com>
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

3 participants