Add function to add calibration exposures

[weaverba137]

As requested by desihub/desisim#273, this PR adds a function to add calibration exposures to the output of desisurvey.progress.Progress.get_exposures().

[sbailey]

This add_row logic makes me nervous because it is fragilely dependent upon exactly which columns exist and in what order they appear in the input exposures table, thus creating a tight coupling with desisurvey.progress.Progress.get_exposures() and making it harder to use outside of that context. How about calling add_row with a dict, where you have filled in the keys of the dict only for columns that actually exist in the input exposures table? If that suggestion doesn't make sense, ping me and I'll put together a code example that might be clearer.

[weaverba137]

I'm pretty sure I understand what you mean here, & that's reasonable.

[sbailey]

I made one minor change to use if 'EXPID' not in exposures.colnames: instead of if 'EXPID' not in exposures to avoid this warning of a future error:

FutureWarning: elementwise == comparison failed and returning scalar instead; this will raise an error or perform elementwise comparison in the future.

One more request: In the future we'll want the flexibility to have arcs and flats with different exposure times (even if they both default to 10 sec for now). Darks will have exposure times >>10 sec (e.g. 1000 sec) and zero will have 0. Otherwise looks good. Thanks.

[sbailey]

One more thing: add_calibration_exposures() has the side-effect of adding an EXPID column to the input exposures table, but those exposure IDs don't match up with the exposure IDs in the output table that also has calibration exposures. I'd suggest either:

1. don't modify the input exposures table at all, and add the EXPID column to output only after copying the structure of the input exposures; or
2. go whole hog and define this function as modifying the input exposures table by both adding an EXPID column and augmenting it with calibration exposures.

I'm fine with either one as long as it is documented. If you prefer (2), I'll note that the implementation could be made much faster by directly adding calibration rows to the input exposures table with dummy EXPID values, sorting by MJD at the end, and then do the final update with meaningful EXPIDs in order. The current runtime of 2 minutes for the full survey isn't too bad, so that shouldn't be the motivating factor for choosing (1) vs. (2).

I'm testing desihub/desisim#275 (wrap-newexp updates) now.

[weaverba137]

I think the way it is set up now, it is no longer necessary to add the 'EXPID' column to the input. I'll look at that.

[sbailey]

Another small thing: the calibration MJDs are added in reverse chronological order compared to the calibration EXPIDs. Please fix and add a test that after adding the calibration exposures:

1. all EXPIDs are unique
2. all MJDs are unique
3. if sorted by MJD, the EXPIDs are in strictly increasing order

It would be fine to start assigning the calibration exposures at 1pm local time and walk forward from there based upon exposure times and readout overheads. If the user requests more calibration exposures than can fit in the time window before the first observation of that night, throw an error.

[weaverba137]

OK, I think I've fixed all the "one more things":

• Input table is not modified.
• MJDs and EXPIDs are guaranteed unique and monotonic increasing by construction, unless the input is not sorted by MJD.
• Exposure times are set based on calibration flavor.

[weaverba137]

Just waiting for tests to complete now.