joshspeagle
commented
3 years ago I was working on this issue earlier this morning, but let me see if what you have is good enough for now. Will review it shortly.
Closed segasai closed 3 years ago
joshspeagle
commented
3 years ago I was working on this issue earlier this morning, but let me see if what you have is good enough for now. Will review it shortly.
segasai
commented
3 years ago I'm happy with other ways of fixing it, I've just urgently needed to plug this hole. (there could be additonal bugs there)
joshspeagle
commented
3 years ago Okay, I actually like your fix better than what I had implemented (mine was done internally without the use of args using the internal live point states). The ability to pass args or lists could be useful to build on in the future if we want to make more changes, plus it just makes everything more explicit (e.g., flagged by method) and avoids more duplication. I'm happy to merge this in once tests have completed. I didn't see signs of any bugs either.
coveralls
commented
3 years ago | Changes Missing Coverage | Covered Lines | Changed/Added Lines | % | ||
|---|---|---|---|---|---|
| py/dynesty/nestedsamplers.py | 51 | 64 | 79.69% | ||
| <!-- | Total: | 55 | 68 | 80.88% | --> |
| Files with Coverage Reduction | New Missed Lines | % | ||
|---|---|---|---|---|
| py/dynesty/nestedsamplers.py | 2 | 67.91% | ||
| <!-- | Total: | 2 | --> |
| Totals | |
|---|---|
| Change from base Build 865353717: | -0.1% |
| Covered Lines: | 3379 |
| Relevant Lines: | 4698 |
segasai
commented
3 years ago Okay, then I'm happy with the merging, given the tests succeeded. I've added a bit more comments as well (no need to wait for the tests from those)
I've needed an urgent fix to #250 since my codes were crashing regularly, so here is a possible fix. I'm not sure that's the best possible fix, but I think it does the job. Basically it adds optional arguments for propose_*() And for the propose_live it accepts a subset of indices. It also avoids the repeated loglikelihood calls. There is a bit of a leakage of abstractions, as an if statement is needed to provide different arguments to propose functions depending on which sampler method is used, but maybe it can be refactored later. The patch include some white space changed due to yapfing of previously un-yapfed nestedsamplers.py The patch is based on the #257 commit branch.