Loocv objectbased

[jelmerborst]

Ready to go, I think.

LOOCV works as explained. Basically, build a model, build an LOOCV object and fit it:

model_3ev = hmp.models.EventModel(n_events=3)
loocv3ev = hmp.loocv.LOOCV(model_3ev, quick=False)
lkh_loocv, modelfits_loocv = loocv3ev.fit(base_data, cpus_cv=cpus, cpus_model=1)

Any type of model is accepted.

@GWeindel : one change I made is that Event.transform() returns the sum of the likelihoods, as that is what we optimize on. In fact, I think that estim_probs_groups should already return the sum (I added the lkh per group to the eventprobs structure). What do you think? If you agree, I can change it, and remove a bunch of .sum()'s from EM. But perhaps there's a reason for having an lkh per group that I'm missing.

[jelmerborst]

Btw, I will add tests tomorrow, and hopefully a tutorial.

[jelmerborst]

Tests added, please check my question in the first comment. If you agree I can add that in a separate PR.

[GWeindel]

I'm going to take a close look next week , do you have a tutorial also? Otherwise no worries I'll just use the tests you wrote.

As per your question I do like that we have them per group now, to me it's useful info to keep e.g. if we want to compare models we can also inspect the group likelihoods change (maybe a group benefits from sharing but not the other, etc.). The fact that there are a few sums is to me not a problem

[jelmerborst]

I'm going to take a close look next week , do you have a tutorial also? Otherwise no worries I'll just use the tests you wrote.

I'll add a tutorial today or tomorrow. I hope :).

As per your question I do like that we have them per group now, to me it's useful info to keep e.g. if we want to compare models we can also inspect the group likelihoods change (maybe a group benefits from sharing but not the other, etc.). The fact that there are a few sums is to me not a problem

Good point. I still think it's a bit misleading – and requires a whole bunch of sum's indeed –, as we're optimizing over the summed lkh. What do you think of the solution of having _estim_probs_group() returning the summed likelihood and adding the likelihood per group as and attribute to eventprobs?

[GWeindel]

Yes but isn't that already what we have after _estim_probs_group?

[jelmerborst]

Added the tutorial @GWeindel , should be ready to go.

[GWeindel]

Looks good to me, I still wish to spend some time testing how good the LOOCV is at least on simulated data but that's a matter for later.

I did the conflict resolution and removed the examples from loocv.py as this should be part of the documentation. Given that it's illustrated in the tutorial I would say this is enough. If you think otherwise you can just re-eimplement it.

So to me it's ready to merge, nice work!

[GWeindel]

OK there are a few more conflicts than I expected, will resolve soon

[GWeindel]

Update: because the upcoming changes in #244 are on the lower level functions of this PR we need to first integrate #244 before merging this one (otherwise each PR makes less sense).

I'll try to do that soon but in the meantime I'll create a release for 1.0.0-b4 as quite a few people need it to be stable and we need a bit of time to see how the new estimator module fits before creating a new beta release for that and LOOCV. But anyone can already use LOOCV from the development branch as soon as it is also merged

[GWeindel]

Sorry Jelmer I lost pace with the recent other PRs so I could not adapt yet.

This still needs to be adapted to the new estimator architecture shipped with #244 of interest probably is the log_likelihood method from events.py. I can give it a go but not in the next few weeks because of vacation + conference.

[rickdott]

Made a first pass with merging to the current devel branch. The LOOCV test passes, but I did not yet consider the new estimation interface. I wonder if we need to, since the model (and thus Estimator) is defined outside of LOOCV, so it should be model-agnostic?

Additionally, I kept the parallelization over folds for now, I think the memory-safe and similarly fast (in situations I can think of) option is to run cpus_cv=1 and cpus_model>1, using my parallelization implementation. We could thus also remove the parallelization over folds here. Feel free to adjust.

I also wonder whether re-fitting models on differing subsets of participants causes issues with max_duration in estim_probs(), like in #323 . Here, some subsets will naturally have the longest trial be longer than others.

[GWeindel]

Thanks Rick! Do you really think the parallelization within a model can be faster than over folds even when these folds include basedata creation?
I still do not like having more than one parallelization strategy but here it might be worth it don't you think?

[rickdott]

In most normal usecases parallelizing over folds will be faster, though probably use more RAM? In case of inbalanced folds paralellizing within fold could be faster. Lets say there's 9 participants with 200 trials, and 1 with 2000. All cpus will be waiting for the 2000 trial participant to finish, while in the within-fold parallelization case there will be no waiting.

I'm more than fine with keeping both parallelizations! I think the current implementation handles it well.

[GWeindel]

OK good thanks for your answer. Let's keep this one then