Composition over Inheritance for Hierarchy classes + Default MH Updaters

[TeoGiane]

This PR introduce a major code re-factoring for the Hierarchy classes. In order to further maximize the amount of code that can be reused, we changed the code pattern for hierarchies, for which the polymorphic behavior is now achieved via composition.

To be precise, each hierarchy can be seen as the composition of a Likelihood, with obvious meaning (thus managing the state of the hierarchy, I/O methods etc.) and a PriorModel, which manages the hyperparameters and prior of the hierarchy, their update and so on.

This code choice preserves code readability, the front end API for the hierarchies whilst allowing a huge simplification in defining a new Hierarchy, once the Likelihood and the PriorModel are already available.

Moreover, this new pattern allows a separate implementation of the update algorithm for the hierarchy. At this stage, thanks to @mberaha contribution to this branch, we have several metropolis updaters that can be used as default update mechanism in a new hierarchy (while allowing possible ad-hoc Gibbs-Sampling strategies). This could be a major simplification for end users, which are now not required to implement the strategy for the hierarchy, since they can rely on default updaters.

Tasks yet to accomplish

• [x] Finish the porting of LinRegUniHierarchy
• [x] Finish the porting of LapNIGHierarchy
• [x] Finish the porting of FAHierarchy

[mberaha]

Well this looks like a lot but if you get a closer look @TeoGiane changed only 64 files! 😂😂😂

[TeoGiane]

Well this looks like a lot but if you get a closer look @TeoGiane changed only 64 files!

Thanks @mberaha, I have not detailed this fact: several changes were needed to use Stan automatic differentiation in Metropolis updaters. To do so, you need to include <stan/math/rev.hpp> and this should be done BEFORE <Eigen/Dense> and <stan/math/prim.hpp>. Using directly the <stan/math/rev.hpp>, automatically include the aforementioned libraries.

[TeoGiane]

@mberaha With the last three commits I have changed how posterior hypers are managed. Now, we have a SemiConjugateUpdater class which handles updaters in which the prior and posterior hyperparameters have the same structure. In this way, we manage post_hypers only in those updaters.

To do so, I had to change some backend interface, now posterior parameters are passed from PriorModels to Updaters via protocol buffers. This is still not perfect, as I would like to set the use of prior hyperparameters as default behavior of PriorModel::sample(). Before doing so, I would like an opinion.

[mberaha]

This is still not perfect, as I would like to set the use of prior hyperparameters as default behavior of PriorModel::sample(). Before doing so, I would like an opinion.

I see two ways:

1) create an overloaded sample method that receives no input, and just calls sample(prior_params)

2) use boost::optional here. Here is a simple example on the usage

#include <boost/optional/optional.hpp>

int foo(boost::optional<int> val = boost::optional<int>()) {
  if ( val ) 
    return *val + 2;

  else return foo(5);
}

TEST(optional, misc) {
  ASSERT_EQ(foo(1), 3);
  ASSERT_EQ(foo(), 7);
}

I think 1) is both easier and more readable!

[brunoguindani]

Part 4 out 4 of four, that is, the last one! I really like the new high-level structure. I have just a small question: why putting the states in a subfolder of hierarchies/likelihoods rather than hierarchies? Also, after dealing with all commented lines, please mark conversations as resolved, or even delete the message, so that this conversation is cleaner.

Now, if you will, I'm gonna go and sleep for 2 days.

[TeoGiane]

With these last commits I gave a general clean to the whole code (thanks to @brunoguindani suggestions). Some stuff still needs to be done, I will go as quickly as possible and keep you up to date as I add more commits.

[TeoGiane]

Tasks accomplished from the last review:

• [x] I have introduced the to_proto() method in some States class. This makes code more readable, as suggested by @mberaha
• [x] I have introduced a new callable class, named covariates_getter, which is used to avoid code repetitions in *_grid methods in Hierarchy and Likelihood classes, as suggested by @brunoguindani.
• [x] Removed test_mh_updater from the parent folder, as it is no more necessary (we can think about a possible different location, if we want to preserve it).

Things still left behind:

• [x] Clean the code I commented during these last commits (which is still to let you see the changes).
• [x] Data-driven initialization for FAHierarchy (still commented)
• [x] Gamma-Gamma example

Things I would split in future PRs

• Documentation
• Improve the States classes and give them a more rigid structure (this is my idea);
• Avoid back and forth proto conversions as suggested by @mberaha in a still unresolved conversation;

[brunoguindani]

Does the code run fine? If so, I'm fine with merging this, then polishing with other PRs. Before that, please make sure the Gamma-Gamma example works -- I see the conversation is still unresolved

[TeoGiane]

Sure, @brunoguindani, I will update the TODO list, so that you will check what is left behind before merging.

[TeoGiane]

Upon your approval, this merge will be pushed to hier-comp-plus-updaters branch inside bayesmix-dev, in order to manage future developments before moving definitively to master.