multisource

[lionel-rigoux]

Some functions of the toolbox are not compatible with multisource and will fail, bug, or display inconsistent results if called on multisource models, often because if only relies on the options.binomial flag instead of using the options.source structure:

• VBA_designEfficiency
• VBA_EKF
• VBA_getDefault
• VBA_getMCMC_predictive_density_fb
• VBA_GN
• VBA_Initialize
• VBA_MFX
• VBA_onlineWrapper
• VBA_ReDisplay
• VBA_summary
• VBA_summaryMFX
• VBA_VolterraKernels

I started a branch fix-binomial to tackle those issues.

Moreover, some redundancy in the code could be avoided, as multisource coded can sometimes handle subcases that are currently treated by separate function (VBA_IPhi_XXX in particular). Also, some features are currently incompatible with multisource although there is no principled reason for this except for the time necessary for implementing them in multisource, namely the one associated with the flag options.UNL and options.nmog (not mentioning stochastic inversion).

[jdaunize]

I would leave multisource inversion routines for cases which rely explicitely on multisource data. This is because, until the full VBA inversion capabilities (cf., e.g., stochastic inversion) can be handled in multisource mode, there is too much to lose to remove these redundancies. Now, some of the functions you are listing here can be either adapted for multisource, or modified to return an error signal when called in multisource mode (temporary fix)...

[lionel-rigoux]

I know that adapting all those functions is a long term objective. I just wanted to point out that a lot of them will fail with multisource without explaining why, and it should be a good idea to at least warn the user that the functions cannot handle multisources.

For the VBA_Iphi functions, I would however immediatly fuse VBA_Iphi, VBA_Iphi_binomial and VBA_Iphi_extended as they return exactly the same results as the latter can handles the first two cases. The main reason is that all those functions are starting to diverge (typo corrections, reordering of blocs of code) and the more we wait, the more difficult the merge. You can have a look at the branch "fix-binomial" to see my intention. And there is nothing to loose. We will keep all functionalities, and have fewer lines of code to maintain...

[jdaunize]

I disagree about fusing these things. This is because we do not know whether later changes may not break e.g., stochastic inversion, which also uses VBA_IPhi...

[lionel-rigoux]

Ok, I will revert the changes in the "fix-binomial" branch. However, I still don't see how this could affect the stochastic inversion. The only functions that are critical for stochastic inversion are the VBAIX*** ones. By definition, the IPhi step should work similarly irrespective of how the states are estimated. Practically, IPhi_extended performs exactly the same compuations as Iphi (for one gaussian source) and Iphi_binomial (for one binomial source). Moreover, IPhi extended not only deal with multisources but also with single-source multinomial data. In the end, the execution tree is very hard to follow.

A minima, I would prepare the removal of the options.binomial field in favour of the source structure (as in "fix-binomial"). Code switches are sometimes depending on tests on sources, sometimes on binomial flag, and guessing which core function is called in which case is rather tricky. I understand that setting the binomial flag is simpler, but we could use a dedicated function as syntactic sugar for source setting, something like options = set_observation_distribution(options, 'gaussian')

[lionel-rigoux]

I am closing this issue follwing the merge of the pull request #45 that handle the problems related to the binomial flag. I will open another issue to reboot the discussion about the simplification of the GN tree.