search

grd349 / PBjam

A repo for our peak baggin code and tips on jam
MIT License
17 stars 6 forks source link

MvN - Adding a multivariate Normal prior #243

Open grd349 opened 4 years ago

grd349 commented 4 years ago

I have finally added the MvN prior that I had been threatening to do. Turns out it is fast (expected).

I have polished the code a little and the docs should be up to @nielsenmb 's standards (largely in part to ctrl-c ctrl-v).

We should run some tests in different corners of the numax parameter space but I am mildly optimistic.

Note - I have not attempted to account for the observational uncertainty on the prior values (the same is true for kde) as this softens things up a little.

I cannot persuade the tests to pass. I have a problem that in pbjam_tests the kde type is hard coded in as a type but with no methods. This causes 2 tests to fail - I'm not really sure how to get round this! What do there tests actually do if the kde class is just a shell with no methods?

@nielsenmb - could you look at the failing tests and see how they could be fixed?

nielsenmb commented 4 years ago

Regarding the testing, I tried to make a set of generic class instances for standardizing the class method tests.

Not sure it worked out so well though.

I'll have a look at it.

On Wed, 1 Jul 2020 at 20:55, Guy Davies notifications@github.com wrote:

I have finally added the MvN prior that I had been threatening to do. Turns out it is fast (expected).

I have polished the code a little and the docs should be up to @nielsenmb https://github.com/nielsenmb 's standards (largely in part to ctrl-c ctrl-v).

We should run some tests in different corners of the numax parameter space but I am mildly optimistic.

Note - I have not attempted to account for the observational uncertainty on the prior values (the same is true for kde) as this softens things up a little.

I cannot persuade the tests to pass. I have a problem that in pbjam_tests the kde type is hard coded in as a type but with no methods. This causes 2 tests to fail - I'm not really sure how to get round this! What do there tests actually do if the kde class is just a shell with no methods?

@nielsenmb https://github.com/nielsenmb - could you look at the failing tests and see how they could be fixed?

You can view, comment on, or merge this pull request online at:

https://github.com/grd349/PBjam/pull/243 Commit Summary

  • Added MVN to priors
  • Polishing MvN class

File Changes

Patch Links:

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/grd349/PBjam/pull/243, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEJWO35BM3QOVRJYLRUCE6TRZOIETANCNFSM4ON52HGQ .

grd349 commented 4 years ago

Super - Thanks @nielsenmb

grd349 commented 4 years ago

Any progress on this? @nielsenmb

nielsenmb commented 4 years ago

Can you copy in the error message you get from pytest?

On Mon, 20 Jul 2020 at 10:37, Guy Davies notifications@github.com wrote:

Any progress on this? @nielsenmb https://github.com/nielsenmb

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/grd349/PBjam/pull/243#issuecomment-660919434, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEJWO32WEE2NVOQGCBOMJ3DR4QF6PANCNFSM4ON52HGQ .

nielsenmb commented 4 years ago

It looks like this branch doesn't have the Travis update. The tests are still run by Pytest.

I'd suggest merging the latest PR #243 into master and then pulling that into the MvN PR.

We should maybe talk about this tomorrow if there's time after the cico.

On Mon, 20 Jul 2020 at 10:55, Martin Nielsen < martinbonielsen.astro@gmail.com> wrote:

Can you copy in the error message you get from pytest?

On Mon, 20 Jul 2020 at 10:37, Guy Davies notifications@github.com wrote:

Any progress on this? @nielsenmb https://github.com/nielsenmb

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/grd349/PBjam/pull/243#issuecomment-660919434, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEJWO32WEE2NVOQGCBOMJ3DR4QF6PANCNFSM4ON52HGQ .

nielsenmb commented 4 years ago

Managed to get a look at the branch, and the pytest failure doesn't look directly related to the MvN addition.

All tests are running fine on master, so if you pull that into the MvN PR then it might fix the issue.

On Mon, 20 Jul 2020 at 11:09, Martin Nielsen < martinbonielsen.astro@gmail.com> wrote:

It looks like this branch doesn't have the Travis update. The tests are still run by Pytest.

I'd suggest merging the latest PR #243 into master and then pulling that into the MvN PR.

We should maybe talk about this tomorrow if there's time after the cico.

On Mon, 20 Jul 2020 at 10:55, Martin Nielsen < martinbonielsen.astro@gmail.com> wrote:

Can you copy in the error message you get from pytest?

On Mon, 20 Jul 2020 at 10:37, Guy Davies notifications@github.com wrote:

Any progress on this? @nielsenmb https://github.com/nielsenmb

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/grd349/PBjam/pull/243#issuecomment-660919434, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEJWO32WEE2NVOQGCBOMJ3DR4QF6PANCNFSM4ON52HGQ .

grd349 commented 4 years ago

Thanks - I'll follow you advice when I have a sec ... :)

What is the time line for merging this into master - after publication?

nielsenmb commented 4 years ago

Yeah, that's probably best, although technically what the referee should be evaluating (if they evaluate the code at all) is what's on Pypi.

On Mon, 20 Jul 2020 at 11:45, Guy Davies notifications@github.com wrote:

Thanks - I'll follow you advice when I have a sec ... :)

What is the time line for merging this into master - after publication?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/grd349/PBjam/pull/243#issuecomment-660952020, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEJWO3YTC4JJZV5EC6D5OTDR4QN57ANCNFSM4ON52HGQ .

nielsenmb commented 2 months ago

Don't think this is relevant anymore, can it be closed?