New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixed the way random walker starts are picked over a boundary #89
Conversation
f6c6d66
to
e6d7682
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, nice catch. I think a test calling _boundary_spread
with some vales and comparing to expected would be really nice.
I think this approach still assumes that the parameter bounds are close to the best value. For things like emission measure I've often made the bounds very large (like 1e-5 to 1e20). The Why not make |
Co-authored-by: Shane Maloney <shane.maloney@dias.ie>
9d17375
to
65050a3
Compare
…oundary for MCMC analysis.
… hard-coding the wrong one.
Codecov ReportBase: 54.63% // Head: 54.86% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #89 +/- ##
==========================================
+ Coverage 54.63% 54.86% +0.22%
==========================================
Files 19 19
Lines 3115 3115
==========================================
+ Hits 1702 1709 +7
+ Misses 1413 1406 -7
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
* Fixed the way random walker starts are picked over a boundary for an MCMC run. * Added test for the method that starts walkers off randomly over the boundary for MCMC analysis. * Now using numpy for the largest 32-bit integer rather than, mistakeny hard-coding the wrong one.
The way I did this in the first place was stupid. Originally the parameter space bounds were multiplied by 100 and then I choose random integer values (why do I want to make everything an integer!?) within the new scaled bounds. This is obviously not right.
The new way is done as L+(U-L)*R, where L and R are the lower and upper bounds for the parameter space and R is a random value between 0 and 1. This should now give values that make sense no matter the bounds.