afeinstein20 / eleanor

A tool for light curve extraction from the TESS FFIs.
MIT License
91 stars 39 forks source link

Quality flag clash #180

Open ethankruse opened 4 years ago

ethankruse commented 4 years ago

Hi,

I don't know how closely you've been following the data release notes, but in S14, the TESS team introduced a new quality flag at the next available unused bit (bit 13 = 2**12 = 4096). At the time it was a per CCD scattered light flag. Then in S20, they started determining scattered light contamination on a per-target basis and setting that bit 13 flag individually. They also started using bit 15 (16384; no idea what bit 14 is used for) to mark fully saturated cadences. And finally, bit 16 was added to mark cadences where there's not enough data for cotrending basis vectors.

To add to this, they stopped using bit 12 (original scattered light prediction) on short cadence targets.

My questions:

  1. You say in your paper that you use bit 13, 4096 for an eleanor quality flag. It was unused at the time, but now overlaps with the per-target scattered light flag. A quick search of the code tells me that you're still using this, but you may want to consider changing it, probably to a much higher bit (bit 30 or something) to avoid future clashes as TESS adds more quality flags.

  2. Relatedly, I can't quite figure out where all you set quality flags, but these lines are worrying. By just adding your flags to the TESS flags, if both have the 4096 bit set, I believe that'll end up rolling over and making bit 13 0 but setting bit 14 to 1. It's not the worst thing, but means that people can't only, e.g., filter out cadences with bit 13 set and get accurate results. https://github.com/afeinstein20/eleanor/blob/8ce0d9bc16e69bdbccf9547d1c87f68a1df02907/eleanor/ffi.py#L152-L157 Generally speaking, the safer way to combine flags is with bitwise or | E.g. taking integer arrays with bits 0-4 individually set and also setting bit 1 (if not already set), you just use x | (2**1)

    >>> x = 2**np.arange(5)
    array([ 1,  2,  4,  8, 16])
    >>> x | 2
    array([ 3,  2,  6, 10, 18])

    So the above file could just be temporarily "fixed" with return flags | pm_flags, but I don't know if that is actually where quality flags are set or if it's the only place.

  3. With bit 12 (predicted scattered light per CCD) being removed from the short cadence targets, and bit 13 (targeted scattered light per target) being used instead, if you only get your quality flags from a single short cadence target per CCD (which last I checked is what I thought was going on), then you're going to be setting an entire CCD's scattered light quality flags based on the single short cadence target, which not only uses its spatial location, but also only sets that flag based on the star's magnitude. Do you also use the FFI quality flags (they only use 3 flags for FFIs: 3, 6 and 12 (Course Point, Reaction Wheel Desaturation Events and Predicted Stray light)?

benmontet commented 4 years ago

Hey, thanks for pointing this out, we had not seen that the quality flags had been extended. We'll make this change (although between my teaching and Adina's preparing for candidacy there's not much time for development atm).

At the time we built those lines the "predicted stray light" FFI flag was not used at all, it was only 3 and 6 that were ever set, which is what inspired us to use the short cadence information. Do they now do a decent job at accurately capturing the stray light in the FFIs?