Python Hierarchy

[taguhiM]

This is a draft implementation of the python NNIG hierarchy. We implemented the necessary metods in python in 'fun.py' which are then called from within python_hierarchy.cc located in pybmix/pybmix/core/pybmixcpp as was done the serialized_collector.cpp by Mario. We migrated all the model-specific methods of a hiearachy to python (except update_hypers at the moment).
Couple of questions/observations:

• the speed of the run_mcmc is dropping significantly, (well, because Python is slow); running Neal3 algorithm with Python hierarchies, DP mixing takes 25 minutes with 500 iterations; for non-conjugate models we suppose it will be even slower considering that also sampling from the full conditionals will be done in python;
• even though you can find implementation of clear_summary_statistics in python since it is kind of model specific method by nature, we are not sure if it is necessary, given that it slows down the runtime;

To try the new python hierarchy it is needed to build and compile inside the pybmix/pybmix/core folder, where there is the CMake file, then it can be run with the run_python.sh file located in the same path.

Note that we also changed some things in the bayesmix submodule, which is the forked repo of bayesmix. Particularly, we updated proto files for python hierarchy and changed the mersenne twister engine to 32 bit version.

Do you have any suggestions for more efficient implementation? If you approve the approach we can go on implementing more generic structures.
It is crucial for us (due to scholarship reasons) to finish the project in the summer exam sessions. To have an estimation of the time and work needed to finish the project; your feedback at the moment is very important. Let us know what you think about our approach and what needs to be changed.

Cheers!

P.S. The pre-commit hooks and styling/naming conventions are not followed yet since in this draft PR we still need to understand if the approach in general is accepted.

[mberaha]

Thanks!

So I'm mainly wondering what is the bottleneck here and if there is a way to circumvent it.
Can you run your code with a profiler so that we see which functions are eating the runtime?

[mberaha]

Also, Neal3 is surely the most computationally intensive algorithm. What happens with Neal2 or BlockedGibbs?

[mberaha]

Actually, I would delegate the computation of posterior hypers entirely to the "draw" python function, which should just receive the data allocated to this cluster for the moment

[taguhiM]

Honestly, we didn't understand this comment, can you please explain more in details?

[mberaha]

Right now, you have two functions: compute_posterior_hypers and draw in fun.py

In practice, it's hardly the case that users will want to implement conjugate hierarchies in python. Therefore, I suggest working directly as if the hierarchy is non conjugate, and have the users implement a draw_prior and a sample_full_conditional pythonfunction. The second one could also call the draw_prior (as in this case) but this logic would be within the python code, and therefore be handled only by the user

[mberaha]

I would believe that there is a faster way than appending, perhaps already reserving the correct size?

[brunoguindani]

This might help: https://pybind11.readthedocs.io/en/stable/advanced/cast/stl.html

[taguhiM]

Thank you. We had changed the same thing in another function but hadn't noticed here. It affected a lot apparently. Now it runs much faster both with Neal2,3. 1100 iterations in 97 seconds for Neal3.

[mberaha]

I think that the MAKE_OPAQUE functionality in the docs should really help with the performance, then.

Also, note that you should not need to convert between py::list and std::vector, just pass the vector of doubles to the function and pybind should take care of the conversion.

[mberaha]

Let us know when you have further updates. I still would like you to run the code with a profiler (compile with debug mode though!) to see where optimization should be focused

[taguhiM]

Thank you. We modified the code to use the pybind11 built-in type conversions.
We will run the profiler in the upcoming days and will share the results.

About the make_opaque, we're still in the reading phase :)

[gbpollam]

Hi,
we tried experimenting with MAKE_OPAQUE to pass the state vector by reference.

We turned std::vector into an opaque type (called DoubleVector) and managed to implement the push_back method in the corresponding class declaration. When overloading the access operator [] however we encountered the first problem: it worked for elements outside the vector as well, returning random numbers.

Moreover, since MAKE_OPAQUE deactivates the automatic conversion between python lists and std::vector we changed the state data structure into a generic pybind11 object, which would then be passed a DoubleVector object. We encountered some problems however in passing it by reference to python.

Finally it appears to us that most of the pybind11 documentation deals with running c++ from python, and not viceversa (as in our case). Can you point us towards some working examples of MAKE_OPAQUE related to what we're trying to do?

[mberaha]

I am not an expert on this. One option would be to open an issue in the pybind11 github and see if a developer answers!

[gbpollam]

Hi, after receiving the answer on the pybind11 github we were finally able to make it work. However, it doesn't make the code any faster (actually it seems to slightly slow it down). This is probably because the vectors we are passing are of extremely small size.
You can find the implementation in the py_hier_opaque branch, is it better to keep this approach (which maybe will make code faster in the event someone needs to pass bigger vectors) or to use the automatic copy conversions?

[mberaha]

Ok, that's interesting!

I don't have a clear answer to that, maybe the best thing is to benchmark the approaches on a multivariate case. We're not interested in anything fancy.
I would do a mock-up where you don't code all the stuff properly. E.g. the lpdf evaluator might return simply the sum of data (even if this does not make any sense) and the sampler might just sampler randomly from a normal distribution without any meaning. Just to benchmark the conversion speed!

[EnricoPaglia]

Hello, we started to work on profiling our code. We tried different profilers but in the end the one that seemed more interpretable to us was gprof (which is also the one prof. Formaggia showed us during the course). You can find attached the results we obtained, we still have to think about it but we thought you could give us some helpful tips on how to interpret it. Thanks
profiler.txt

[mberaha]

Hello, we started to work on profiling our code. We tried different profilers but in the end the one that seemed more interpretable to us was gprof (which is also the one prof. Formaggia showed us during the course). You can find attached the results we obtained, we still have to think about it but we thought you could give us some helpful tips on how to interpret it. Thanks
profiler.txt

So it appears that 16% of the time is spent in creating pybind arrays:

pybind11::array::array<double>(pybind11::detail::any_container<long>, pybind11::detail::any_container<long>, double const*, pybind11::handle) 

which takes more than twice the time compared to the second most expensive function.

[mberaha]

For the moment, I would suspend optimization (maybe open the issue on pybind11's GitHub) and move on to having the example working via the python interface.

In the end, users should be able to sample from the model using the MixtureModel. run_mcmc function

[taguhiM]

We are currently trying to run the new hierarchy from python. So far we have encountered a lot of problems with the installation of the library. In our fork we have updated the bayesmix and we are using our fork of bayesmix as a submodule. To summarize, we are getting 2 types of errors:

• Errors with TBB stuff, specifically sometimes it doesn't find the tbb_2019_U8 directory inside the bayesmix/lib/math/lib or it happens that it is replaced by the tbb directory, but inside that it can't find the cmake directory.

-- Using math TBB
    CMake Error at pybmix/core/pybmixcpp/bayesmix/CMakeLists.txt:28 (file):
      file COPY cannot find
      "/home/taguhi/polimi/project_bayesmix/pybmix/pybmix/core/pybmixcpp/bayesmix/lib/math/lib/tbb_2019_U8":
      No such file or directory.
    
    
    CMake Error at pybmix/core/pybmixcpp/bayesmix/CMakeLists.txt:30 (include):
      include could not find requested file:
    
        /home/taguhi/polimi/project_bayesmix/pybmix/pybmix/core/pybmixcpp/bayesmix/lib/math/lib/tbb/cmake/TBBBuild.cmake
    
    
    CMake Error at pybmix/core/pybmixcpp/bayesmix/CMakeLists.txt:33 (tbb_build):
      Unknown CMake command "tbb_build".

• The following error occurs in 2 cases: if we manually copy the tbb stuff to solve the previous error, or it just happens on its own without the proceeding tbb error. This error occurs also when installing the original bayesmix-dev/pybmix library twice (running the pip3 install -e . command twice sequentially). The first installation goes successful, the second gives the same error as we are getting in our fork.

SetuptoolsDeprecationWarning: Custom 'build_py' does not implement 'get_data_files_without_manifest'.
    Please extend command classes from setuptools instead of distutils.
      warnings.warn(
    reading manifest file 'pybmix.egg-info/SOURCES.txt'
    adding license file 'LICENSE'
    writing manifest file 'pybmix.egg-info/SOURCES.txt'
    running build_ext
    CMake Error at CMakeLists.txt:2 (project):
      Running
    
       '/tmp/pip-build-env-ntexs_d2/overlay/bin/ninja' '--version'
    
      failed with:
    
       No such file or directory

We are kind of stuck on this in the last few days :'(

[mberaha]

The first error is likely our fault. I pushed a patch on pybmix/master to fix the CMakeLists

Moreover, at line 93 of the bayesmix/CMakeLists.txt file, in place of #TBB already included, write ${TBB_ROOT}/include

This should fix the include errors.

Let me know if the ninja error persists. If so, we can avoid using ninja I believe

[taguhiM]

The first error is likely our fault. I pushed a patch on pybmix/master to fix the CMakeLists

Moreover, at line 93 of the bayesmix/CMakeLists.txt file, in place of #TBB already included, write ${TBB_ROOT}/include

This should fix the include errors.

Let me know if the ninja error persists. If so, we can avoid using ninja I believe

Seems like it doesn't solve the problem. The error can be reproduced by using the latest versions of our forks. Attached is the full error log.
error_log.txt

[mberaha]

This is likely because you need to delete the build folder and the egg-info one.

Then try to build stuff using cmake in the usual way

mkdir build
cd build
cmake ..
make pybmixcpp

Let me know if this works, in this case we can set aside the setup.py file for a moment and have a hacky workaround.

Also, it looks like you're missing some folders inside the bayesmix submodule. This might be due to the previous installation system (?). You might try to delete your pybmix folder locally and just clone it back.

[taguhiM]

This is likely because you need to delete the build folder and the egg-info one.

Then try to build stuff using cmake in the usual way

mkdir build cd build cmake .. make pybmixcpp

Let me know if this works, in this case we can set aside the setup.py file for a moment and have a hacky workaround.

Also, it looks like you're missing some folders inside the bayesmix submodule. This might be due to the previous installation system (?). You might try to delete your pybmix folder locally and just clone it back.

The error I attached was produced on a fresh clone of the repo and a new environment to avoid any unnecessary errors. I'll try the building stuff now and let you know.

[mberaha]

python3 -m pip

We did as you said.

• mkdir build
• cd build
• cmake ..
• make pybmixcpp
• cd ..
• python3 -m pip install -e .

Attached is the error log. About the sphinx, we followed the official documentation. Different approaches actually, with apt, conda and pip

What about the output of make pybmixcpp? Did it produce any error?

[taguhiM]

python3 -m pip

We did as you said.

• mkdir build
• cd build
• cmake ..
• make pybmixcpp
• cd ..
• python3 -m pip install -e .

Attached is the error log. About the sphinx, we followed the official documentation. Different approaches actually, with apt, conda and pip

What about the output of make pybmixcpp? Did it produce any error?

No it compiles without any warnings or errors. Totally fine.

[mberaha]

What about the output of make pybmixcpp? Did it produce any error?

No it compiles without any warnings or errors. Totally fine.

Then a workaround for the moment is not to build with python3 -m pip but only with cmake and add the following lines before any import of pybmixcpp in Python files:

import os
import sys
sys.path.insert(0, <PATH_TO_BUILD_DIR>)

To obtain the path to the build dir in a smart way, assume that the build dir is always in pybmix/build.
Then i the file "mixture_model.py" you could do something like

HERE = os.path.dirname(os.path.realpath(__file__))
BUILD_DIR = os.path.join(HERE, "../../build/")
sys.path.insert(0, os.path.realpath(BUILD_DIR))

the same should work in "estimators.py" that is the only other file that calls directly pybmixcpp

The main disadvantage is that this does not install the package to the system library, so that you will need to work only in the root folder of pybmix. I will try to have a look at your building issues in the next days.
Please let me know if this workaround fixes your problems

[taguhiM]

What about the output of make pybmixcpp? Did it produce any error?

No it compiles without any warnings or errors. Totally fine.

Then a workaround for the moment is not to build with python3 -m pip but only with cmake and add the following lines before any import of pybmixcpp in Python files:

import os
import sys
sys.path.insert(0, <PATH_TO_BUILD_DIR>)

To obtain the path to the build dir in a smart way, assume that the build dir is always in pybmix/build. Then i the file "mixture_model.py" you could do something like

HERE = os.path.dirname(os.path.realpath(__file__))
BUILD_DIR = os.path.join(HERE, "../../build/")
sys.path.insert(0, os.path.realpath(BUILD_DIR))

the same should work in "estimators.py" that is the only other file that calls directly pybmixcpp

The main disadvantage is that this does not install the package to the system library, so that you will need to work only in the root folder of pybmix. I will try to have a look at your building issues in the next days. Please let me know if this workaround fixes your problems

This approach seems to not be working because seems like the proto files were being generated ('converted') in the setup.py build_extention along with pybmixcpp. Maybe we need to run that two_to_three_command separately somewhere?

[mberaha]

You're right. You can achieve this in dozens of ways. The simplest one that comes to my mind is to have a shell script that does it, then call it by hand since you should not modify the protos at any rate.
You can also bundle it in the CMake file, see https://stackoverflow.com/questions/39960173/run-custom-shell-script-with-cmake

[taguhiM]

You're right. You can achieve this in dozens of ways. The simplest one that comes to my mind is to have a shell script that does it, then call it by hand since you should not modify the protos at any rate. You can also bundle it in the CMake file, see https://stackoverflow.com/questions/39960173/run-custom-shell-script-with-cmake

To make a shall script is straight-forward. WHat I'm wondering is how is that two_to_three_command even working? I have 2 doubts:

1. The official documentation of 2to3 says:
   To translate an entire project from one directory tree to another use:
   $ 2to3 --output-dir=python3-version/mycode -W -n python2-version/mycode
   But you're using PROTO_OUT_DIR only, actually PROTO_IN_DIR is never used.
   two_to_three_command = [py2to3, "--output-dir={0}".format(PROTO_OUT_DIR), "-W", "-n", PROTO_OUT_DIR]
   Why is that even working?
2. From the setup file I understand it is used for proto fies only but 2to3 works only on py files. I'm trying manually to use it and it works only on the init.py.

So I'm guessing, there is something else happening before that I don't see. Maybe its this?

        if not os.path.exists(self.build_temp):
            os.makedirs(self.build_temp)

What am I missing?
Sorry, maybe too many questions recently, but the setup is a little messy and the proto stuff especially is not straight-forward.

[mberaha]

So in the cmake file we first compile proto files into python classes which are written in PROTO_OUT_DIR.
The problem is that protoc generates python2 classes. Therefore, we need to call 2to3 to convert from python2 to Python3 classes.

Basically, in the lines of code you flagged, i am converting all the proto classes from python2 2 to Python3. Since the input and the output Path coincide, 2to3 will overwrite the old classes with the new ones.

So before running 2to3, you should build stuff with cmake so that it compiles the protobuf messages into python classes. Maybe is this step you're missing?
If not, i will find a shell script that does the compilation proto->python2 and the conversion altogether

[taguhiM]

Clear now. Thanks. Hopefully next time I'll bother you just to say it works :)

[taguhiM]

Here come the good news! The very first working example of Python hierarchy.
Library built manually with cmake. In the convert_proto.sh file there is an environment variable which needs to be set manually. Since this approach is temporary as you said, it is as well.

The example can be found in docs/examples/test_run.py

[mberaha]

Here come the good news! The very first working example of Python hierarchy. Library built manually with cmake. In the convert_proto.sh file there is an environment variable which needs to be set manually. Since this approach is temporary as you said, it is as well.

The example can be found in docs/examples/test_run.py

Great news indeed!!

So far, it seems like the fun.py path is hardcoded in the C++ source file py_global.cc. Do you think there is a way to avoid doing this? I can imagine a user wanting to try out different possibilities, so that instead of fun.py one might have normal_kernel_functions.py, then gamma_kernel_functions.py and so on. Then upon calling PythonHierarchy() users can specify the path of the functions file. Do you think it is possible?

Another possibility is to have a meta-programming approach where we write the py_global.cc file from a python script, and recompile the library each time!

[taguhiM]

Hi.
We've got a problem here. Turned out, it was early to celebrate and it doesn't actually work. Apparently, we were compiling only bayesmix part and not the files we added alongside to it. And since python is good at hiding errors, we didn't notice that.
Now we're stuck on CMake problem for several days. The problem is that we want to build and compile bayesmix alongside with the python_hierarchy.cc/h and the other a few files we have added. We thought it was easy to do by just using a 'copy' of the bayesmix cmake outside and modifying the paths in it. Buuut, it doesn't.
Do you have any advice on this? We're becoming desperate.
pybmixcpp
--- bayesmix
--- pyhton_hierarchy.cc/h
--- py_global.cc/h
--- etc.
--- CMakeLists.txt (this is where we thought it should work)

We get all types of problems, sometimes with protobuf, then we fix it, then with tbb, then we fix it, then with something else. Then also we managed to compile without errors, but inside python couldn't import the AlgorithmWrapper (not found): core dumped, tried to play with paths, but didn't seem to work anyway. We pushed this version in a new branch called 'Cmake_hell'

[mberaha]

Sorry I'm not following, when you said it was working, you didn't run the example? Or by some magic the example was working and now it's broken?

I tried compiling your stuff and it compiles fine, but I get an ImportError:

ImportError: cannot import name 'AlgorithmWrapper' from 'pybmix.core.pybmixcpp' (unknown location)

The thing you can do is to locate the exact source of the error: does everything work on master? Does it work if you comment enough lines of code? (Like, all the new stuff?) Does the c++ executable you had before still work?

Another thing to try is to go back to a very simple example, starting from pybind11's cmake example, and adding a simple function that computes like an addition written in python, called in C++ and then called from Python just as we're doing now

[gbpollam]

Sorry I'm not following, when you said it was working, you didn't run the example? Or by some magic the example was working and now it's broken?

I tried compiling your stuff and it compiles fine, but I get an ImportError:

ImportError: cannot import name 'AlgorithmWrapper' from 'pybmix.core.pybmixcpp' (unknown location)

The thing you can do is to locate the exact source of the error: does everything work on master? Does it work if you comment enough lines of code? (Like, all the new stuff?) Does the c++ executable you had before still work?

Another thing to try is to go back to a very simple example, starting from pybind11's cmake example, and adding a simple function that computes like an addition written in python, called in C++ and then called from Python just as we're doing now

Hi, to clarify:
test_run.py was running and producing a plot, but we didn't notice at first that it was just the histogram of the data. Our python_hierarchy wasn't being compiled at all.
How should we modify the CMakeLists.txt files in order for them to be properly compiled alongside bayesmix?
One approach we tried was creating a subfolder in the pybmixcpp folder containing the bayesmix folder, python_hierarchy.cc/h , py_global.cc/h, load_hierarchied_2.h and the cmakelists.txt. The idea was to have them all compiled together from the pybmixcpp CMakeLists.txt, but it didn't work.

Do you have any suggestion?

[mberaha]

Just add them to the pybind11_add_module directive in the main CMakeLists.txt file of pybmix, just as algorithm_wrapper and serialized_collector

[taguhiM]

Well, that didn't work as well. It compiles, but can't find the "AlgorithmWrapper' and it's not a path issue as it seems.
Anyway, somehow, with some magic mess now it seems to work, there are still some stuff to fix. We'll keep you updated. The new code is in the branch Cmake_experiments_2

[mberaha]

Well, that didn't work as well. It compiles, but can't find the "AlgorithmWrapper' and it's not a path issue as it seems.
Anyway, somehow, with some magic mess now it seems to work, there are still some stuff to fix. We'll keep you updated. The new code is in the branch Cmake_experiments_2

Ok great! I've opened a PR #14 proposing a new structure for the package. It splits the package in two and has more maneagable CMakeLists.txt files. If you try it out and it works for you, then we can convert this PR to be in that format

[taguhiM]

Very good news !
We finally got a working version! And found a way to organize the files and the Cmake (after a week of being stuck on this).
You can try to compile and run the version in this branch

You have to clone directly that branch, since the structure of the folders is different, git doesn't allow to checkout (this can be solved after merging). To run the examples, just run the docs/examples/test_run.py file.

We propose to keep this structure if you don't mind.
We get segmentation fault (core_dumped) in the end of the script, it's still work in progress, most probably some python-c++ destructor issue.

[mberaha]

Keep te package/file structure you're most comfortable with. There is always time to change later in any case!

Once everything works, i would close this PR and open a new one from the right branch.

One last thing that you should make sure to check is how to deal with multiple fun.pys file, meaning that it would be great if the path could be passed at runtime. Otherwise, it would be nice to have a meta-programming approach that recompiles stuff if a new file is passed, or something like that

[gbpollam]

Keep te package/file structure you're most comfortable with. There is always time to change later in any case!

Once everything works, i would close this PR and open a new one from the right branch.

One last thing that you should make sure to check is how to deal with multiple fun.pys file, meaning that it would be great if the path could be passed at runtime. Otherwise, it would be nice to have a meta-programming approach that recompiles stuff if a new file is passed, or something like that

We have implemented a way to decide which fun.py to use at runtime (so far only for one method: initialize_state). It works as follows:

• There is a "main" fun.py, which implements all the necessary methods. Each method reads the name of the module we want to use from an environment variable (e.g. fun1.py), imports that module and calls the corresponding implementation of the function in the newly imported module.
• Whenever we want to use a different module (hierarchy) in test_run.py, we just need to change the environment variable through os.environ['TRUE_NAME'] = 'fun1'

Please refer to this branch for a working example. Let us know if you like the approach and if we should expand it to the other methods.

[mberaha]

I'm all in for that! Especially since we can hide this from the end-users and handle this in an internal function (I hope, otherwise we just write very good docs!!)

[gbpollam]

I'm all in for that! Especially since we can hide this from the end-users and handle this in an internal function (I hope, otherwise we just write very good docs!!)

Hi, we've extended the approach to all the other methods (of the non-conjugate case).
Moreover we have created a new hierarchy_changer.py file in pybmix/core implementing the function change_hierarchy, which hides from the user the modification of the environment variable. Now the user only has to import that file in test_run.py and call the function passing the new name as a string, in order to change which hierarchy they want to use.
Was this what you had in mind when you said to handle this in an internal function?

[mberaha]

Yes exactly what I had in mind!