search

gemini-hlsw / lucuma-odb

5 stars 0 forks source link

Twilight Again #1443

Closed swalker2m closed 2 weeks ago

swalker2m commented 2 weeks ago

1432 added sequence generation for twilight calibration observations. Unfortunately it failed to take into account that these observations use degenerate targets which don't pass observation validation. This PR updates GeneratorParams to carry deferred target errors instead of stopping immediately when the target isn't a valid science target.

Before: ITC inputs required

case class GeneratorParams(
  itc:             GeneratorAsterismParams,
  observingMode:   ObservingMode,
  calibrationRole: Option[CalibrationRole]
)

After: ITC inputs may be missing

case class GeneratorParams(
  itcInput:        Either[ItcInput.Missing, ItcInput.Defined],
  observingMode:   ObservingMode,
  calibrationRole: Option[CalibrationRole]
)

At sequence generation time, depending on the calibration role, the lack of a valid target may or may not be an actual issue. In the case of a twilight, missing ITC parameters can be ignored.

Disclaimer

I'm not at all happy with the error handling. I want to circle back and clean this up if I can, but apparently this is needed for a demo on Wednesday so I'm offering it early.

cquiroz commented 2 weeks ago

It seems this would be transparent to clients?

swalker2m commented 2 weeks ago

It seems this would be transparent to clients?

For sequence generation, missing target parameters should only be visible for non-twilight observations. For observation validation, if we ran them for twilight, you would see the error. I recall that we don't show validation errors for calibrations?

cquiroz commented 2 weeks ago

we don't have validation errors for calibrations

swalker2m commented 2 weeks ago

I'll merge this so we can get on with testing the Twilight sequence itself, but I'm already working on refactoring the missing parameter handling. There's a lot of duplication there that we can remove, I think.