Likelihood from config file

[cdcapano]

Have likelihood evaluator class be initialized from the config file. This also moves waveform generation set up into the LikelihoodEvaluator's from_config function. This is the first step to moving the sampler initialization parameters into the config file, to eventually allow for more sampling engines.

The only change to the ini file is you now need:

[likelihood]
name = gaussian

in the config file, and the likelihood option is removed from gwin. This will also allow for specifying parameters for the likelihood class in the config file. For instance, you will be able to specify the mean and covariance for the test_normal likelihood class.

I've also added a DataBasedLikelihoodEvaluator class to make the separation between the analytic likelihoods and the likelihoods that use data / a waveform generator cleaner.

I'm not crazy about the names of the classes (never really did like LikelihoodEvaluator), and I know this has caused some people confusion. Suggestions for better class names are welcome.

Marking as work in progress, as still have to finish and test. Also, #14 should be merged first; then I'll update this patch accordingly.

[pep8speaks]

Hello @cdcapano! Thanks for updating the PR.

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

Comment last updated on June 06, 2018 at 14:37 Hours UTC

[cdcapano]

@duncanmmacleod That "pep8 speaks" thing is going to make PRs difficult to read. Is there any information in there that's not in code climate? If not, can we quiet that?

[ahnitz]

Yeah, I agree with Collin on this. It's going to be a mess if it is adding comments.

…

On Wed, Apr 11, 2018, 12:08 Collin Capano ***@***.***> wrote: @duncanmmacleod <https://github.com/duncanmmacleod> That "pep8 speaks" thing is going to make PRs difficult to read. Is there any information in there that's not in code climate? If not, can we quiet that? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#17 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACGrRqOjTkSEOHjQ6vSxiD_ocLwcA2NKks5tndY0gaJpZM4TPuos> .

[coveralls]

Pull Request Test Coverage Report for Build 119

• 57 of 129 (44.19%) changed or added relevant lines in 2 files are covered.
• 1 unchanged line in 1 file lost coverage.
• Overall coverage decreased (-1.3%) to 39.214%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
gwin/option_utils.py	2	5	40.0%
gwin/likelihood.py	55	124	44.35%

Files with Coverage Reduction	New Missed Lines	%
gwin/likelihood.py	1	60.64%

Totals
Change from base Build 113:	-1.3%
Covered Lines:	998
Relevant Lines:	2545

---

💛 - Coveralls

[cdcapano]

Ok, I think this is ready for review now.

[cdcapano]

@cmbiwer @vivienr poke

[cdcapano]

@vivienr @cmbiwer poke x 2

[vivienr]

PR #2123 has been merged into the latest release, so we could just import it.

[cdcapano]

@vivienr Done. @duncanmmacleod Since this requires the latest release of pycbc, I've added version requirements for pycbc to setup.py and requirements.txt.

[vivienr]

While not really within this PR, we could add here an example on how to use simulated noise?

[cdcapano]

The BBH example is on simulated noise. See lines 396 and 397 of gwin.rst.

[cmbiwer]

Hey, sorry. I don't have a problem with this PR. But I'll leave it unreviewed by me since I see Vivien has some comments/review.

[cdcapano]

@vivienr Ok, I think I addressed all of the issues.

[vivienr]

Looks good, thanks !