Field rotation update - Githubissues

desihub / desimeter

DESI coordinates and transformations

BSD 3-Clause "New" or "Revised" License

2 stars 4 forks source link

Field rotation update #172

Closed julienguy closed 3 years ago

julienguy commented 3 years ago

Updates to the empirical field rotation model based on new data saved in py/desimeter/data/guide_20201215_with_hexrot.csv.

schlafly commented 3 years ago

Do you want a "code" review here or me to actually check the quality of the fits, etc.? My impression from your plots was that things looked as well as could be expected---though I don't have a good sense for what we expect the error in the rotation rate measurements to be.

julienguy commented 3 years ago

Just having a rapid second look at the code to double check there is not some gross mistake. It's probably going in operation tomorrow evening, so it's better to have a second pair of eyes on it.

schlafly commented 3 years ago

Looks good to me; I don't see any issues. Minor notes:

as we've discussed, the terms that are odd in x all got much smaller from v1 to v2. I might try deleting them from the fit until additional data show they're necessary.
I'd be inclined to try to get rid of the isinstance checks from the body of the code and push them to the beginning and end with something like scalar = np.isscalar(ha) if scalar: ha = np.atleast_1d(ha) dec = np.atleast_1d(dec) ... intervening code can assume that everything's a numpy array ... res = rotation_rate_arcsec_per_min if scalar: res = res[0] return res

julienguy commented 3 years ago

I leave the asymmetric HA terms in because I am not sure we have a strong reason to remove them and they are small. For reference, the analysis report associated to this PR is https://desi.lbl.gov/DocDB/cgi-bin/private/ShowDocument?docid=6032.