Ompi: Regression in MPI_Info_delete() with missing key

[dalcinl]

This is with branch jjhursey:explore/dlopen-local which should be (very close to) current master plus the dlopen fixes.

>>> from mpi4py import MPI
>>> info = MPI.Info.Create()
>>> info.Delete("key")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "mpi4py/MPI/Info.pyx", line 86, in mpi4py.MPI.Info.Delete
mpi4py.MPI.Exception: MPI_ERR_INTERN: internal error

Previous versions of Open MPI failed with MPI_ERR_INFO_NOKEY.

[jjhursey]

Due to change made in PR #3743

[jsquyres]

Looks straightforward to fix; I'll take a stab at it.

[dalcinl]

@jsquyres After almost every new release of Open MPI, I found regressions by running mpi4py's testsuite. You know, I'm so maniac as to test the error code value after attempting to delete a missing key from an Info object.

Would it be possible to somehow get mpi4py testsuite run as part of the CI infrastructure? I proposed this in the past, and I got comments like we may not be interested in mpi4py bugs. I'm not claiming that my codebase (and its very few dependencies) is bug-free, but from experience on the last 10 years, the mpi4py bug rate is almost null. Most of the bug reports and issues we get are always related to build issues because of broken user environment, dynamic linking issues related to the infamous dlopen situation, of bugs in the backend MPI implementation. Most of mpi4py's testsuite is plagued of special-casing to disable tests that are know to fail as Open MPI releases appeared over the years. I'm now at a point were most of mpi4py tests pass, and it would be good to keep things that way for the future to come :-).

mpi4py is thoroughly tested on Linux and macOS with MPICH and Open MPI, and on Windows with Microsoft MPI, all of this using public CI services (Travis CI, Circle CI, AppVeyor, Bitbucket Pipelines). Yes, sh.. happens, a future bug in mpi4py could eventually slip and disrupt Open MPI testing infrastructure, but I think the risk outweighs the benefits. I'm quite confident that mpi4py should help a lot in preventing regressions like the one reported in this issue.

[jsquyres]

@dalcinl BTW, I don't know if you know how much we value your tests and bug reports. If we haven't said it before, THANK YOU!

This sounds like quite an interesting idea. We've actually been beefing up our CI infrastructure quite a bit recently (particularly on PR builds). How long does the mpi4py test suite take to run? How many np does it need?

@bwbarrett FYI.

[dalcinl]

@jsquyres Take a quick look at https://travis-ci.org/mpi4py/mpi4py/ This runs in a container were some dependencies (Python and NumPy) are preinstalled. It takes less than two minutes to test Open MPI. If you inspect any of the logs, in each build I test twice: one with 1 process, and a second one with 5 processes. Of course your own mpi4py testing does not need to use all these Python versions, you should just test let say Python 3.6 (most recent Python release).

Look also to this other one: https://circleci.com/gh/mpi4py/mpi4py/34 Individual builds are still under 2 minutes, this is installing all dependencies using the Anaconda Python distribution and conda-forge packageds which provides pre-built packages (including Open MPI, which I'm a maintainer of) for all Python-related dependencies you may need.

[jsquyres]

@dalcinl That sounds great.

@bwbarrett Any chance you have a minion who could add this to the OMPI CI?

[ggouaillardet]

@dalcinl thanks for the (many) bug reports you submitted !

@jsquyres @bwbarrett should this be in MTT rather than CI ?
i remember i tried a bit, but unfortunatly mpi4py was not straightforward to integrate within the MTT infrastructure.
at first, we need a stable version of mpi4py (that obviously includes its test suite).
the rationale is virtually no-one in the Open MPI community wants to sort which bugs are from mpi4py, and which bugs are from Open MPI, so let's have a mpi4py and its test suite that are known to work.
then, the way MTT works is it runs one test at a time.
iirc, there is no straightforward way to get the list of tests (i wrote a patch for mpi4py in order to achieve this, but that was long time ago). once we get this, MTT integration should be pretty easy.

[dalcinl]

@ggouaillardet @jsquyres @bwbarrett Right now I'm working on overhauling mpi4py's testsuite. Eventually, I'll provide a flag to the test script to not skip any know failure (some tests are currently alsways disabled for Open MPI), that way you can be sure to run everything. Regarding stability, I'd be making a new mpi4py release very soon, but anyway current master is very stable.

[bwbarrett]

@dalcinl, is the test suite single node or multi node? Currently, that (and runtime, but 2 minutes is pretty short) is the main driver between CI and MTT. We don't have a great story yet for running multi-node jobs in CI. It's on the todo list, but down a ways. Either way, yes, we should at least run an experiment with regular automated testing with mpi4py.

[dalcinl]

@bwbarrett I usually run on a single node with 1 and 5 processes (all mpi4py's CI services run that way currently). The testsuite should also run on multiple nodes, but I've not tested it in a long time, probably some MPI I/O tests would fail on multiple nodes, as I create temporary files in '/tmp' (this should be easy to fix if required in the future).

[dalcinl]

@bwbarrett Look at this script https://bitbucket.org/mpi4py/mpi4py/src/master/conf/ci/anaconda.sh

With minimal modifications (to not install the Open MPI package in the conda install ... line), you get all full run of the testsuite. This script should be sourced and then test-package should be executed. Some other adjustments are needed, like the ANACONDA variable. As long as mpicc/mpiexec are found in PATH, the script is ready to build and test in a container that previously build and install Open MPI.

[bwbarrett]

I opened https://github.com/open-mpi/ompi/issues/4158 to track the testing request. Since this bug is fixed, closing it.