search

sot / chandra_aca

Chandra Aspect Camera Tools
https://sot.github.io/chandra_aca
BSD 2-Clause "Simplified" License
0 stars 0 forks source link

Update to 2018-11 drift model #64

Closed taldcroft closed 6 years ago

taldcroft commented 6 years ago

Same as #63 but back-ported to ska-legacy.

jeanconn commented 6 years ago

This seems reasonable. As far as I can tell we don't have any other things in the master branch that would need to be in ska-legacy for this to work, and we should get the rest of those fixes with the upgrade to ska3 soon.

taldcroft commented 6 years ago

Stand by, I remembered I did not explicitly test this in Ska.

jeanconn commented 6 years ago

GRETA ska is different anyway, so I'll try to test in the VM or devska before we release to Mark for the windows build.

taldcroft commented 6 years ago

This branch passes tests (setup.py test ) now in Ska. Because we're doing a release I decided to git cherry-pick 04875b4 to get the AGASC 1.7 fix.

jeanconn commented 6 years ago

Ok. I was willing to ignore those AGASC 1.7 related fails (along with all the agasc module test fails) but might as well update.

taldcroft commented 6 years ago

Yeah, it is was just a "I can fix this in 30 seconds" thing.

This also passes Ska2-linux32 on quango except for errors in test_residuals.py that seem to indicate an old version of mica installed. Maybe that's not surprising.

jeanconn commented 6 years ago

What's the plan for this for HEAD flight Ska? It looks like I started to setup starcheck to re-confirm the offsets, so it does call the drift code. But I don't think I every got around to adding a real check that the independent calculation from starcheck matched the dynamic aimpoint file (which probably would have been illuminating 2 weeks ago). But in the end I think that just means that the current values of the drift model calculated in starcheck are saved to some JSON and ignored.

But I'm assuming that I should still test against this and xija for a starcheck release? Looking like I won't have that for today's LR and more for next week.

taldcroft commented 6 years ago

Yes, test with this and xija for the next release.