CaltechOpticalObservatories / NGPS

NGPS Software
2 stars 0 forks source link

Allow empty RA/DEC fields ; define calibration behavior in OS, GUI #8

Closed astronomerdave closed 4 months ago

astronomerdave commented 1 year ago

This is how we signify a data set which does not require moving the telescope, E.G., bias frame, etc.

jenniferwillow commented 1 year ago

I have changed the defaults to NULL for RA and DEC and it now skips the attempt to construct the RA or dec object and doesn't populate the RA and DEC degrees automatically. It appears to work but we should test this build extensively to see if there are any cases that break things. So far everything I've done has worked as expected after this change was made.

astronomerdave commented 1 year ago

re-opening and adding @chazshapiro as additional assignee for the following reasons: It's true that you can create a line with an empty RA,DEC, but if you then try to save it, two things happen:

  1. the OTM complains
  2. the GUI then deletes the offending row

Furthermore, while you can create a new row w/o RA, DEC, you cannot edit a row to remove RA,DEC and in the console is displayed, java.lang.RuntimeException: Expected a string of the form hh:mm:ss.sss, but got: ''

chazshapiro commented 1 year ago

do we have a list of target names that are code for some calibration procedure?

astronomerdave commented 1 year ago

do we have a list of target names that are code for some calibration procedure?

Not yet but that was envisioned.

chazshapiro commented 1 year ago

I will have to implement at least one, say "CALIBRATE", as a way to have the OTM understand blank coordinates

chazshapiro commented 1 year ago

We need to define what a legal CALIBRATE row looks like and what it means. Which columns are required/optional? What do we expect the system to do? This is a conversation for the group.

I'm going to leave the OTM as-is until this is defined - it will report an error for blank RA/DEC, which is the correct behavior for non-CALIBRATE rows.

jenniferwillow commented 1 year ago

I'm going to leave this open because it discusses the CALIBRATE issues that aren't related to a null RA and Dec. Null RA and Dec is now fully supported and it required changing the database so nulls are allowed for those fields.

chazshapiro commented 1 year ago

I've updated OTM to allow blank RA/DEC for some target names corresponding to calibrations: BIAS, ARC, FLAT, DARK, ETALON. To calculate slew times, it assumes blank RA/DEC will use the last non-blank coordinates above it in the list. The time spent on calibration is just the exposure time from the target table. If there are additional delays, we need to agree on how to model them.

SEE UPDATE 2024-01-27

chazshapiro commented 1 year ago

I updated so that calibration rows do not cause flags or delays during daylight. If the list starts with a bunch of calibrations, they will schedule immediately. The first sky target must start after sundown, so there may be a delay if calibrations finish before then.

chazshapiro commented 9 months ago

The OTM can have blanks for RA and DECL, but when those are blank in the GUI, the GUI is sending a CSV file with the word "null" in those fields. When any table entry in the GUI is blank, can the CSV contain a blank?

chazshapiro commented 9 months ago

I've added a kludge to the OTM so that it runs a find/replace on the CSV file to get rid of "null" everywhere and replaces the original file. It's using "sed" in the shell. Ideally let's save the file in the needed format.

chazshapiro commented 9 months ago

Bug #1: When I load a sample CSV file and then try to edit it, the GUI stops me from deleting the RA or DECL, e.g. to change the row to a calibration. The workaround is to insert a fresh row and not enter RA or DECL. See the screenshot, including the bottom of the black terminal showing the GUI feedback.

Bug #2: OTM complains when RA or DECL is non-blank in a calibration row. I'll either allow this or else make the complaint more explicit. Update: NOW FIXED (allowed)

Screenshot 2023-09-26 at 9 09 11 AM
chazshapiro commented 5 months ago

I've updated the OTM so that rather than signifying calibration rows with particular reserved target names, it will treat any target name starting with "CAL_" as a calibration and thus allow blank RA/DECL. E.g. CAL_DARK, CAL_BIAS, etc.

chazshapiro commented 4 months ago

I think the original issue is fixed, and we've developed some follow on bugs