Mutation python

[BBArrosDias]

Creating a python package that binds mutation++ routines.

I created a folder interface that contains:

1. the Fortran wrapper that was in the src
2. the Python package

The Fortran wrapper was tested with the stagline solver reproducing the same results as the previous architecture.

The python package contains unit testing and an air_11 equilibrium example in the folder test/.

[codecov[bot]]

Codecov Report

Merging #155 (5034a1c) into mutation_python (acc42b9) will not change coverage. The diff coverage is n/a.

@@               Coverage Diff                @@
##           mutation_python     #155   +/-   ##
================================================
  Coverage            70.07%   70.07%           
================================================
  Files                  134      134           
  Lines                 8451     8451           
================================================
  Hits                  5922     5922           
  Misses                2529     2529           

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update acc42b9...8216210. Read the comment docs.

[rdbisme]

Hello, this review will take a while for me since I have zero bandwidth at the moment 😉

[leroyvn]

My 2 cents: If you want to ship pybind11 alongside Mutation++, I'd suggest to use a Git submodule instead of dropping in sources, unless there is a strong reason to do otherwise. You'll have an easier time updating the code afterwards and your Git diffs will be briefer.

[BBArrosDias]

Thanks a lot, @leroyvn. Would you like to review the code? There is some function that could be improved, with your feedback.

[leroyvn]

Sure (I actually recognised some of the draft Python bindings I wrote back when I was at VKI!). @rubendibattista the files that require a review are at the bottom and top of the diff (Fortran at the top, Python at the bottom). I'll stick with general advice and opinion for now (my knowledge of pybind11 is quite shallow, actually). I'll try to have a deeper look later.

I really think it's good to go for Git submodules: updating is actually critical sometimes (e.g. currently, outdated pybind11 code will result in embarrassing errors if used with Python 3.9.0) and submodules make the process a lot easier.

My choice for unittest as the testing suite is not one I would recommend now: I find pytest to be much more usable for people who don't want to know about fixtures and stuff. However I'm not sure if the tests which are still left are really relevant if you know what you're doing with pybind11: they will likely break if the underlying C++ code changes, so they don't test only the binding code. Still, if you want to write tests, the best is AFAIK to reproduce C++ tests with the Python bindings: they will break when the C++ tests will break, so repairing both should be pretty easy.

Docstring generation is also quite painful. At the moment, you have to maintain both the docstrings and the corresponding Doxygen comments, which makes it possible for both to go out of sync. If you want to fix that, the pybind11_mkdoc utility can be helpful.

[rdbisme]

My 2 cents: If you want to ship pybind11 alongside Mutation++, I'd suggest to use a Git submodule instead of dropping in sources, unless there is a strong reason to do otherwise. You'll have an easier time updating the code afterwards and your Git diffs will be briefer.

On this subject, I'd also say to not ship pybind, but require it just as dependency. It's task of the user to download it with their own favorite package manager.

[rdbisme]

I really think it's good to go for Git submodules: updating is actually critical sometimes (e.g. currently, outdated pybind11 code will result in embarrassing errors if used with Python 3.9.0) and submodules make the process a lot easier.

As I said before, I'd say to not ship pybind at all. Configuring the CI in the right way will cover problems against new versions of pybind... Otherwise we risk to accumulate technical debt (as it's already with Catch2, for example)

My choice for unittest as the testing suite is not one I would recommend now: I find pytest to be much more usable for people who don't want to know about fixtures and stuff. However I'm not sure if the tests which are still left are really relevant if you know what you're doing with pybind11: they will likely break if the underlying C++ code changes, so they don't test only the binding code. Still, if you want to write tests, the best is AFAIK to reproduce C++ tests with the Python bindings: they will break when the C++ tests will break, so repairing both should be pretty easy.

+1 on pytest

[leroyvn]

As I said before, I'd say to not ship pybind at all. Configuring the CI in the right way will cover problems against new versions of pybind... Otherwise we risk to accumulate technical debt (as it's already with Catch2, for example)

Then there are instructions to use pybind11 with PyPI, conda-forge, vcpkg, Homebrew, and countless Linux distros. The big pro to submodules IMO is that you reduce the required amount of testing (no risk of people not having the required library version) and can more easily push updates. Cons are as you said @rubendibattista.

That being said, if you package your code properly (really not an easy thing IMO, though), you can actually require pybind11 as a build dependency as part of your setuptools process. This brings a lot of safety (you're no longer dependent on the host OS to have timely pybind11 updates) and retains most of the flexibility allowed by submodules, provided that you don't need to modify your dependencies (very unlikely indeed with libs like Eigen, Catch2 or pybind11).

But be warned: Python packaging and dependency management is still a wild jungle. Properly managing requirements requires care and failure to do so will likely cost you a lot sometime in the future. Proceed with great care! If you want to go that way, I can share my experience.

[jbscoggi]

@BBArrosDias it seems that the new python tests don't work. I guess this is a problem with the configuration of the tests. Checkout this for more info. Maybe some python packages need to be installed?

[pietroparodi]

Hello, Pietro here. Just wanted to report that I managed to install the BBarrosDias:mutation_python branch. I did encounter two difficulties:

• I had to run as root sudo make install, because permissions are needed by the installer to put the Python module in /usr/local/lib/python3.6/dist-packages/. Maybe the target installation directory could be changed.
• The installation stops with the following error:

  
  CMake Error at src/cmake_install.cmake:54 (file):
  file INSTALL cannot find
  "/home/pietro/Documents/Mutation++/Editing/Mutationpp/build/build/lib.linux-x86_64-3.6/libmutation++.so".
  Call Stack (most recent call first):
  cmake_install.cmake:44 (include)

Makefile:73: recipe for target 'install' failed make: *** [install] Error 1

but somehow by running the `sudo make install` command again, the installation completes without errors.

I checked that mutationpp can be imported, and `interface/python/tests/Mutation.py` also runs successfully after installing the `pytest` module.

[rdbisme]

Ehi @pietroparodi. Thanks for reporting. Just a fast feedback on your feedback: never use sudo when installing python module 😏. If it's the only way, well, it's a broken way! :)

So thanks for reporting this. I delivered my PhD manuscript, and I feel the energy coming back up in my veins. I'll maybe play a bit with the @BBArrosDias branch. :)

[adelval1]

Hello mutant peoples. I managed to install the BBarrosDias:mutation_python branch as well. I did it in a different way from @pietroparodi but ended up with the same errors which I was able to overcome thanks to his directions here! :)

I'll write here all I did that is related to the branch and not particular issues I had with my own python set up. For the record, I use python 3.6.4 in a MacBookPro 2017, not the default python 2.7 that comes with Mac.

I compiled and installed mutation++ the same way we normally do for which I had to go to ccmake .. to enable the building of the python wrapper. In my case, I didn't need sudo, I used the usual make -j8 install. Once I did this for the first time I had problems with mutation saying it didn't find pybind11 so I went to Mutationpp/CMakeLists.txt and changed add_subdirectory(thirdparty/pybind11) for find_package(pybind11) just like it's done with Eigen3. In my case, I had pybind11 installed using brew. Then I did make -j8 install again and encountered the same problem as @pietroparodi . As he said, I did it again and the problem was gone...I didn't manage to run Mutation.py because of problems with pytest (my own problems I guess), but I can run the example Air_11_Equilibrium.py and my own scripts as well.

[BBArrosDias]

Thanks @adelval1, for playing with the interface.

I compiled and installed mutation++ the same way we normally do for which I had to go to ccmake .. to enable the building of the python wrapper. In my case, I didn't need sudo, I used the usual make -j8 install. Once I did this for the first time I had problems with mutation saying it didn't find pybind11 so I went to Mutationpp/CMakeLists.txt and changed add_subdirectory(thirdparty/pybind11) for find_package(pybind11) just like it's done with Eigen3. In my case, I had pybind11 installed using brew. Then I did make -j8 install again and encountered the same problem as @pietroparodi . As he said, I did it again and the problem was gone...I didn't manage to run Mutation.py because of problems with pytest (my own problems I guess), but I can run the example Air_11_Equilibrium.py and my own scripts as well.

For the case of @pietroparodi, I think the sudo part could be something related to how he installed python. Indeed in my case, I dont need to install with sudo.

About pybind11, we discussed if we should import the package as a third party or not. If I remember correctly, @rubendibattista did something nice for the case of Eigen, so maybe we can do the same for pybind11.

For running two times the make install, I also have the same problem. It seems that when you install the first time, the package is not installed correctly in the python site-packages folder. And by witchcraft, when you do the second, it works. I would say that is something related to the CmakeList in the python interface folder and the setup.py. I tried to follow an example from google (which I cannot find anymore), but I think their case was more straightforward. Maybe @rubendibattista checking those files can be a nice starting point.