Refactor likelihood module

[cdcapano]

The likelihood module is quite long, and will only get longer as people add support for things like ROQs. This breaks the likelihood module up into a package. The base classes are in likelihood/base.py, test models are in likelihood/test_models.py, the classes that assume Gaussian noise are in likelihood/gaussian_noise.py. All of the classes (with the exception of the base classes) are imported in likelihood/__init__.py, so imports in scripts should be the same.

Putting on hold as this depends on #17.

[pep8speaks]

Hello @cdcapano! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on July 06, 2018 at 11:16 Hours UTC

[coveralls]

Pull Request Test Coverage Report for Build 127

• 95 of 136 (69.85%) changed or added relevant lines in 5 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.3%) to 39.476%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
gwin/likelihood/init.py	7	10	70.0%
gwin/likelihood/gaussian_noise.py	65	75	86.67%
gwin/likelihood/analytic.py	20	48	41.67%

Totals
Change from base Build 122:	0.3%
Covered Lines:	1009
Relevant Lines:	2556

---

💛 - Coveralls

[vivienr]

Is the idea that DataBasedLikelihoodEvaluator may be moved to a newly created likelihood/data.py if/when the likelihood/base.py gets too big? And a separate likelihood/roq.py for the ROQ (which can be used on either Gaussian noise or data). Will it be easy enough for the ROQ to access (inherit?) the "gaussian_noise" or "data" codes without having to re-code too much?

[cdcapano]

@vivienr I wasn't planning on moving DataBasedLikelihood, but could if you want, or if we find it's necessary in the future. Yes, I was thinking the roq likelihood would go into its own file, and it shouldn't be any problem for ROQs (or any other likelihood class) to inherit from other classes. This is just a way to make the modules more easier to read so you don't end up with a massively long file.

[cdcapano]

@duncanmmacleod or @cmbiwer any objections to this?

[duncanmmacleod]

@cdcapano, the only vaguely eyebrow-raising thing is the new module name test_models.py which looks to me like a module containing unit tests, and given that it contains classes whose names also match the pytest conventiones, pytest itself might get confused if we point it at the gwin repo. It might be nice to rename that to testmodels.py or toymodels.py, or similar.

I will not object if you decide not to change it, however.

[cmbiwer]

Maybe just call the module analytical or something similar. I'd be in favor of keeping the underscore though if its more than a single words.

Otherwise, I don't have any objections on this PR.

[cdcapano]

@cmbiwer @duncanmmacleod Ok, I renamed test_models analytic.

[cdcapano]

@duncanmmacleod I don't know why Travis is failing... it seems to be complaining about a glue dependency.

[cdcapano]

@vivienr @duncanmmacleod @cmbiwer Could one of you review this so I can merge it? I'd like to branch off of this for the changes to the sampler API.

I know we discussed changing the name from "likelihood(evaluator)" to "(user)models". I figured I'd do that in another PR, to keep PRs manageable.

[duncanmmacleod]

LGTM