desihub / desimeter

DESI coordinates and transformations
BSD 3-Clause "New" or "Revised" License
2 stars 4 forks source link

Posparams covariance #92

Closed julienguy closed 4 years ago

julienguy commented 4 years ago

Adding covariance to the output parameters of fit_posparams in compliance with https://docs.google.com/document/d/1-_OdDwGg889rqjNpbtxNzmn1RTHPfg44HlzwGmHBUIM/edit#

Among several choices, I resorted to

If no uncertainties on the positions are given (which is the case for now), the default consists in using the rms of the residuals to the fit as uncertainties on the measurements to determine the parameters uncertainties. The covariance is based on numeric derivatives of the chi2 done in the routine scipy.optimize.minimize. I am not familiar with this so I don't know how accurate the covariances will be. We'll have to study this a bit more.

joesilber commented 4 years ago

Error at: https://app.codacy.com/gh/desihub/desimeter/file/45481835469/issues/source?fileBranchId=18566628#l156

joesilber commented 4 years ago

Can the modified fitter be called such that it uses the older algorithm that I had written? Do we have quantitative demonstration that the new one works better?

julienguy commented 4 years ago

Can the modified fitter be called such that it uses the older algorithm that I had written? Do we have quantitative demonstration that the new one works better?

If you have in mind the fit of circles to initiate your fit, this was in a previous PR and it indeed helped recover a many failed fits while not introducing new ones. Now I can add an option to not do this initialization if you want.

joesilber commented 4 years ago

Yes, that's what I was wondering. I read the code more careful and see that you're only doing circle fits to find offset_x and offset_y. I was worried if we were trying to do circle fits on the phi arcs, which I think are a scary / noisy path to follow.