search

desihub / desitarget

DESI Targeting
BSD 3-Clause "New" or "Revised" License
18 stars 23 forks source link

add ZWARN bad QA bits #754

Closed sbailey closed 3 years ago

sbailey commented 3 years ago

This PR adds ZWARN bits BAD_SPECQA and BAD_PETALQA and updates the MTL logic to reject those in addition to NODATA. I'll open a companion desispec PR after this to set those bits when making zmtl (formerly zqso formerly zcat) files.

Example desispec outputs are in /global/cfs/cdirs/desi/users/sjbailey/spectro/redux/zmtl, though I have not tested how mtl updates would use them. Unit tests did catch some typos so the updates are at least somewhat tested.

@geordie666 please take a look. I did not update the $ZCAT_DIR environment variable, but let's consider renaming that to $ZMTL_DIR too in both the code and etc/desitarget.module .

Also: I did not remove the zqso code from desitarget yet, even though it is moving over to desispec as part of the aforementioned PR. Let's make sure it fully works there (as zmtl) before removing it from desitarget as a separate PR.

coveralls commented 3 years ago

Coverage Status

Coverage increased (+0.0005%) to 58.761% when pulling e475f84f6429ab2a9aef40388da8f35501b4c40d on zwarn_badspecqa into 263d22ea11f9df428bf6e2e5208b6681c9719312 on master.

geordie666 commented 3 years ago

Just so you know: To test this I've flipped two of the tiles in /global/cfs/cdirs/desi/users/sjbailey/spectro/redux/zmtl/tiles.csv to zdone=true.

geordie666 commented 3 years ago

Oh, and I've now flipped the two tiles I tested in /global/cfs/cdirs/desi/users/sjbailey/spectro/redux/zmtl/tiles.csv back to zdone=false.

sbailey commented 3 years ago

Thanks for the quick review. I'll leave $ZCAT_DIR as-is, and update the change log after merging so that I don't have to wait for tests again.