gemini-hlsw / lucuma-core

Lucuma Core Classes
Other
9 stars 9 forks source link

Negative LST #933

Closed swalker2m closed 3 months ago

swalker2m commented 3 months ago

Add this test case at the end of the ODB's createCallForProposals:

  test("bug") {
    expect(
      user = staff,
      query = """
        mutation {
          createCallForProposals(
            input: {
              SET: {
                type: FAST_TURNAROUND
                semester: "2024B"
                activeStart: "2024-10-01"
                activeEnd: "2024-12-31"
                submissionDeadlineDefault: "2024-08-31T22:00:00Z"
                partners: [{ partner: US }]
                instruments: [GMOS_NORTH, GMOS_SOUTH]
              }
            }
          ) {
            callForProposals {
              id
            }
          }
        }
      """,
      expected = json"""
        {
          "createCallForProposals": {
            "callForProposals": {
              "id": "c-108"
            }
          }
        }
      """.asRight
    )
  }

It will produce a stack trace:

java.time.DateTimeException: Invalid value for HourOfDay (valid values 0 - 23): -4
    at java.base/java.time.temporal.ValueRange.checkValidValue(ValueRange.java:319)
    at java.base/java.time.temporal.ChronoField.checkValidValue(ChronoField.java:718)
    at java.base/java.time.LocalTime.of(LocalTime.java:341)
    at lucuma.core.math.skycalc.ImprovedSkyCalcMethods.setHours(ImprovedSkyCalcMethods.scala:66)
    at lucuma.core.math.skycalc.ImprovedSkyCalcMethods.getLst(ImprovedSkyCalcMethods.scala:47)
    at lucuma.core.math.skycalc.ImprovedSkyCalcMethods.getLst$(ImprovedSkyCalcMethods.scala:17)
    at lucuma.core.math.skycalc.ImprovedSkyCalc.getLst(ImprovedSkyCalc.scala:18)
    at lucuma.core.math.skycalc.ImprovedSkyCalc.getLst(ImprovedSkyCalc.scala:199)
    at lucuma.odb.graphql.input.CoordinateLimitsInput$.lstAt$1(CoordinateLimitsInput.scala:61)
    at lucuma.odb.graphql.input.CoordinateLimitsInput$.defaultRaLimits(CoordinateLimitsInput.scala:63)
    at lucuma.odb.graphql.input.CoordinateLimitsInput$Create$.default(CoordinateLimitsInput.scala:115)
    at lucuma.odb.graphql.input.SiteCoordinateLimitsInput$Create$.default(SiteCoordinateLimitsInput.scala:35)
    at lucuma.odb.graphql.input.CallForProposalsPropertiesInput$.$anonfun$7(CallForProposalsPropertiesInput.scala:89)
    at scala.Option.fold(Option.scala:263)
    at lucuma.odb.graphql.input.CallForProposalsPropertiesInput$.lucuma$odb$graphql$input$CallForProposalsPropertiesInput$Create$$anon$1$$_$applyOrElse$$anonfun$2(CallForProposalsPropertiesInput.scala:89)
    at cats.ParallelArityFunctions.parMap8$$anonfun$1(ParallelArityFunctions.scala:50)
    at grackle.Result.map(result.scala:43)
    at grackle.Result.map$(result.scala:32)
    at grackle.Result$Success.map(result.scala:170)
    at grackle.ResultInstances$$anon$1.map(result.scala:304)
    at grackle.ResultInstances$$anon$1.map(result.scala:304)
    at cats.ParallelArityFunctions.parMap8(ParallelArityFunctions.scala:50)
    at cats.ParallelArityFunctions.parMap8$(ParallelArityFunctions.scala:11)
    at cats.ParallelArityFunctions2.parMap8(ParallelArityFunctions2.scala:11)
    at cats.syntax.Tuple8ParallelOps.parMapN(TupleParallelSyntax.scala:68)
    at lucuma.odb.graphql.input.CallForProposalsPropertiesInput$Create$$anon$1.applyOrElse(CallForProposalsPropertiesInput.scala:103)
    at lucuma.odb.graphql.input.CallForProposalsPropertiesInput$Create$$anon$1.applyOrElse(CallForProposalsPropertiesInput.scala:55)
...

The root cause of which is that our LST calculation produces negative results instead of being confined to the range [0, 24). I notice that we used to wrap this value at one point, but decided better of it without leaving behind any trace of what was happening.

tpolecat commented 3 months ago

This was causing a problem in ITAC that was very hard to track down so I uncommented these lines in OCS back in 2022 https://github.com/gemini-hlsw/ocs/pull/2051 … they can probably be uncommented here as well.

tpolecat commented 3 months ago

Can't believe I remember doing this. Please clap.

swalker2m commented 3 months ago

Can't believe I remember doing this. Please clap.

Wow, 👏 thanks I'll try it.

swalker2m commented 3 months ago

Addressed in #934