desihub / desispec

DESI spectral pipeline
BSD 3-Clause "New" or "Revised" License
36 stars 24 forks source link

Support reading zbest redshift files #2374

Closed weaverba137 closed 1 month ago

weaverba137 commented 2 months ago

Some early tiles in the daily specprod have zbest instead of redrock files. To support loading daily into a database, these need to be read by desispec.scripts.zcatalog. In this PR:

Open questions:

Some additional operational testing should probably be done before merging.

coveralls commented 2 months ago

Coverage Status

coverage: 30.128% (-0.04%) from 30.164% when pulling 46c167a9de6edbfc9d83873329ba2d727aca7ab2 on read-zbest into 97b174838a2e951a32aa719f19da07e184585c39 on main.

weaverba137 commented 2 months ago

At the data telecon on 2024-09-24, there appeared to be a consensus that we should not put additional effort toward recovering data missing from zbest files. That leaves two open questions: does a particular Table() conversion impact performance, and new/unknown columns.

The second open question is also mentioned in desihub/desidatamodel#199.

sbailey commented 1 month ago

I verified with runs of this branch and main, that the switch from numpy structured arrays to astropy Table has negligible impact on the overall runtime for desi_zcatalog and that they produce identical catalogs, i.e. nothing funny with bytes vs. strings, masked values, etc.