sot / starcheck

BSD 3-Clause "New" or "Revised" License
3 stars 0 forks source link

Remove sybase use for star stats #337

Closed jeanconn closed 4 years ago

jeanconn commented 5 years ago

Remove sybase use for star stats

This is a redo of #257 and #187

jeanconn commented 5 years ago

Regarding

https://github.com/sot/starcheck/pull/187#discussion_r179206680

I did compare this current approach (fetching all acq and guide stats and then using non-indexed searches within them) to using the agasc_id indexes on the hdf5 tables (via tables.read_where).

For an average set of stars in a schedule, it looks like using the indexed approach would reduce this part of the runtime by a bit more than a third (2.2 seconds to use the pre-fetch, 1.4 seconds to open the hdf5, keep it open, and do indexed queries). The 0.8 seconds did not seem worth it in this case (though fetching all the stats to start is not particularly memory efficient either).

jeanconn commented 5 years ago

@taldcroft Do you want this to go too or would you like more time to consider?

jeanconn commented 4 years ago

IIRC acqs and guides were put into Table from this suggestion https://github.com/sot/starcheck/pull/257#discussion_r224488994

jeanconn commented 4 years ago

I can add a comment about that if it isn't going to be the convention to just Table things if the code holds still long enough.

taldcroft commented 4 years ago

Thanks for refreshing my memory. Indeed any data coming straight from HDF5 should generally be Table'd so that character data can be handled sensibly. So to answer my question, the string comparison ok = acqs['img_func'] == 'star' does require Table.