Skip to content

Python hierarchy and mixing #15

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

Closed
wants to merge 60 commits into from

Conversation

taguhiM
Copy link
Collaborator

@taguhiM taguhiM commented Jun 10, 2022

In this PR we present the draft version of our work, implementing

  • the python hierarchy;
  • the python mixing;
  • generalized structure for states and hypers;
  • hierarchy and mixing 'switchers';
  • new structure of directories and corresponding CMake files.

*The documentation and styling of the code are not yet done.

We would like to mention that we did try using the 'make opaque' technique to pass by reference between c++ and python, but that surprisingly didn't improve the performance.

To build the library:

$ mkdir build
$ ./make_.sh

To run an example using the python implementations:
$ python3 docs/examples/test_run.py

We would like to hear your comments, questions and advice to improve the code before the final PR (we will need to open one in bayesmix too, since there are minor modifications also there).

@taguhiM taguhiM requested review from mberaha and brunoguindani June 10, 2022 10:33
@mberaha
Copy link
Contributor

mberaha commented Jun 10, 2022

Thanks a lot for the work!

The PR at the moment is too big, you should split it up in smaller chunks so that I can review them separately. Let's start with the part about the Python hierarchy, since I'm already familiar with that.

On a side note, I think that the most critical part will be the overall build system. I noticed some weir stuff:

  • stan math is included in a submodule, but that's already in the bayesmix submodule... please keep only the bayesmix one
  • there are copy-pasted cmakelists files around, it makes life harder for me to figure out what's going on.

In the end, we must be able to build this as one or two python packages (cf #14)

Please let me know when the PR is ready for a first round of review

@taguhiM
Copy link
Collaborator Author

taguhiM commented Jun 11, 2022

Hi. We followed your request and opened a new PR with only the python hierarchy. We also removed the lib/math from submodules (we had added it at some point because of tbb problems but fortunately it is not needed anymore). About the CMake stuff, we followed the "if it's working, don't touch it" rule :). We would like to hear a more in depth comment about that to understand what should be changed.

How would you like us to proceed with the python mixing PR? Should we open a new one without the hierarchy, or we should wait untill this one is merged?

Also, we're starting to work on the report and to not create a clutter in the last days, we would like to send each chapter when it's ready for the review, if you don't mind :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants