igmhub / baofit

Fits cosmological data to measure baryon acoustic oscillations.
9 stars 5 forks source link

toymc-samples mode crash when one realisation has "hit max dilation limit" #19

Closed londumas closed 8 years ago

londumas commented 8 years ago

When doing a "toymc-samples" mode. If one of the realisation has a "hit max dilation limit". The calculus stops when it could just write the results in the *.toymc.dat with a 0 for chi² results and go to the next realisation.

londumas commented 8 years ago

Fixed

mblomqvist commented 8 years ago

Comment on issue fix: Added a try-catch block in the toy MC sampling, such that a runtime error from the dilation limits in the correlation model does not terminate the analysis. Failed fits are printed to the same _toymc.dat output file as the successful ones, giving the initial parameter values followed by 0 for chi^2.

ngbusca commented 8 years ago

Wouldn’t this fix potentially bias the results of toymc by artificially removing the realizations where the dilation parameters need to go beyond the specified limits?

On Dec 17, 2015, at 5:45 AM, mblomqvist notifications@github.com wrote:

Comment on issue fix: Added a try-catch block in the toy MC sampling, such that a runtime error from the dilation limits in the correlation model does not terminate the analysis. Failed fits are printed to the same _toymc.dat output file as the successful ones, giving the initial parameter values followed by 0 for chi^2.

— Reply to this email directly or view it on GitHub https://github.com/deepzot/baofit/issues/19#issuecomment-165418168.

londumas commented 8 years ago

It corresponds to ~ 10% of realisations when min dilation limit = 0.5 max dilation limit = 2.5 For the cross-correlation.

mblomqvist commented 8 years ago

Yes, the failed fits could potentially bias the results, so good judgment is important, as always, when analyzing them. Still, it seemed like the better option to allow the toy MC sampling to finish, and let the user decide what to do with the result.