Skip to content

Commit

Permalink
Don't calculate calibrations for inactive observations
Browse files Browse the repository at this point in the history
  • Loading branch information
cquiroz committed Nov 14, 2024
1 parent 2991848 commit e98b4f0
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,12 @@ object CalibrationsService extends CalibrationObservations {
case (oid, Right(GeneratorParams(itc, mode, _))) if itc.isRight || !requiresItcInputs => (oid, mode)
}

// Find all active observations in the program
private def activeObservations(
pid: Program.Id,
)(using Transaction[F]): F[List[Observation.Id]] =
session.execute(Statements.selectActiveObservations)(pid)

/**
* Requests calibrations params for a program id and selection (either science or calibration)
*/
Expand Down Expand Up @@ -259,7 +265,10 @@ object CalibrationsService extends CalibrationObservations {
tgts <- calibrationTargets(CalibrationTypes, referenceInstant)
gsTgt = CalibrationIdealTargets(Site.GS, referenceInstant, tgts)
gnTgt = CalibrationIdealTargets(Site.GN, referenceInstant, tgts)
uniqueSci <- allObservations(pid, ObservationSelection.Science).map(uniqueConfiguration)
active <- activeObservations(pid)
uniqueSci <- allObservations(pid, ObservationSelection.Science)
.map(_.filter(u => active.contains(u._1)))
.map(uniqueConfiguration)
allCalibs <- allObservations(pid, ObservationSelection.Calibration)
uniqueCalibs = uniqueConfiguration(allCalibs)
calibs = toCalibConfig(allCalibs)
Expand Down Expand Up @@ -356,5 +365,13 @@ object CalibrationsService extends CalibrationObservations {
WHERE c_observation_id = $observation_id AND c_calibration_role IS NOT NULL
""".query(observation_id *: calibration_role *: core_timestamp.opt *: observing_mode_type.opt)

val selectActiveObservations: Query[Program.Id, Observation.Id] =
sql"""
SELECT
c_observation_id
FROM t_observation
WHERE c_program_id = $program_id AND c_workflow_user_state IS DISTINCT FROM 'inactive'
""".query(observation_id)

}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import lucuma.core.enums.DatasetQaState
import lucuma.core.enums.DatasetStage
import lucuma.core.enums.EmailStatus
import lucuma.core.enums.Instrument
import lucuma.core.enums.ObservationWorkflowState
import lucuma.core.enums.ObserveClass
import lucuma.core.enums.ObservingModeType
import lucuma.core.enums.Partner
Expand Down Expand Up @@ -1635,7 +1636,7 @@ trait DatabaseOperations { this: OdbSuite =>
""".asRight
expect(user = user, query = q, expected = expected)
}

def setGuideTargetName(
user: User,
oid: Observation.Id,
Expand Down Expand Up @@ -1676,4 +1677,20 @@ trait DatabaseOperations { this: OdbSuite =>
.downFields("createConfigurationRequest", "id")
.require[ConfigurationRequest.Id]

def setObservationWorkflowState(user: User, oid: Observation.Id, wfs: ObservationWorkflowState): IO[ObservationWorkflowState] =
query(
user,
s"""
mutation {
setObservationWorkflowState(input: {
observationId: "$oid"
state: ${wfs.tag.toUpperCase}
}) {
state
}
}
"""
).map: json =>
json.hcursor.downFields("setObservationWorkflowState", "state").require[ObservationWorkflowState]

}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import lucuma.core.enums.CalibrationRole
import lucuma.core.enums.CloudExtinction
import lucuma.core.enums.GmosAmpGain
import lucuma.core.enums.GmosAmpReadMode
import lucuma.core.enums.ObservationWorkflowState
import lucuma.core.enums.ObservingModeType
import lucuma.core.enums.Site
import lucuma.core.math.Angle
Expand Down Expand Up @@ -1008,5 +1009,31 @@ class calibrations extends OdbSuite with SubscriptionUtils {
} yield ()
}

test("Don't add calibrations if science is inactive") {
for {
pid <- createProgramAs(pi)
tid1 <- createTargetAs(pi, pid, "One")
tid2 <- createTargetAs(pi, pid, "Two")
oid1 <- createObservationAs(pi, pid, ObservingModeType.GmosNorthLongSlit.some, tid1)
oid2 <- createObservationAs(pi, pid, ObservingModeType.GmosSouthLongSlit.some, tid2)
_ <- prepareObservation(pi, oid1, tid1) *> prepareObservation(pi, oid2, tid2)
_ <- setObservationWorkflowState(pi, oid1, ObservationWorkflowState.Inactive)
_ <- withServices(service) { services =>
services.session.transaction.use { xa =>
services.calibrationsService.recalculateCalibrations(pid, when)(using xa)
}
}
gr1 <- groupElementsAs(pi, pid, None)
ob <- queryObservations(pid)
} yield {
val oids = gr1.collect { case Right(oid) => oid }
val cCount = ob.count {
case CalibObs(_, _, Some(_), _, _) => true
case _ => false
}
assertEquals(cCount, 2)
assertEquals(oids.size, 2)
}
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ import lucuma.odb.graphql.query.ExecutionTestSupport
import lucuma.odb.graphql.query.ObservingModeSetupOperations
import lucuma.odb.json.all.transport.given

class setObservationWorkflowState
class setObservationWorkflowState
extends ExecutionTestSupport
with ObservingModeSetupOperations
with ObservingModeSetupOperations
with UpdateConstraintSetOps {


Expand All @@ -41,7 +41,7 @@ class setObservationWorkflowState

def computeItcResult(oid: Observation.Id): IO[Unit] =
query(
pi,
pi,
s"""
query {
observation(observationId: "$oid") {
Expand Down Expand Up @@ -79,32 +79,16 @@ class setObservationWorkflowState
).map: json =>
json.hcursor.downFields("observation", "workflow", "state").require[ObservationWorkflowState]

def setObservationWorkflowState(oid: Observation.Id, wfs: ObservationWorkflowState): IO[ObservationWorkflowState] =
query(
pi,
s"""
mutation {
setObservationWorkflowState(input: {
observationId: "$oid"
state: ${wfs.tag.toUpperCase}
}) {
state
}
}
"""
).map: json =>
json.hcursor.downFields("setObservationWorkflowState", "state").require[ObservationWorkflowState]

/** Test that we can change to this specified state, and then back. */
def testTransition(oid: Observation.Id, state: ObservationWorkflowState): IO[Unit] =
queryObservationWorkflowState(oid).flatMap: current =>
setObservationWorkflowState(oid, state) >>
setObservationWorkflowState(oid, current).void
setObservationWorkflowState(pi, oid, state) >>
setObservationWorkflowState(pi, oid, current).void

/** Test that we can change to the specified states and back, and CANNOT change to anything else. */
def testTransitions(oid: Observation.Id, allowedTransitions: ObservationWorkflowState*): IO[Unit] =
val legal = allowedTransitions.toList
val illegal = ObservationWorkflowState.values.toList.filterNot(legal.contains)
val illegal = ObservationWorkflowState.values.toList.filterNot(legal.contains)
legal.traverse_(testTransition(oid, _)) >>
illegal.traverse_ : state =>
interceptOdbError(testTransition(oid, state)):
Expand All @@ -115,12 +99,12 @@ class setObservationWorkflowState
///

import ObservationWorkflowState.*

test("Undefined <-> Inactive"):
for {
pid <- createProgramAs(pi)
oid <- createObservationAs(pi, pid)
_ <- assertIO(queryObservationWorkflowState(oid), Undefined)
_ <- assertIO(queryObservationWorkflowState(oid), Undefined)
_ <- testTransitions(oid, Undefined, Inactive)
} yield ()

Expand All @@ -134,7 +118,7 @@ class setObservationWorkflowState
tid <- createTargetWithProfileAs(pi, pid)
oid <- createGmosNorthLongSlitObservationAs(pi, pid, List(tid))
_ <- computeItcResult(oid)
_ <- assertIO(queryObservationWorkflowState(oid), Unapproved)
_ <- assertIO(queryObservationWorkflowState(oid), Unapproved)
_ <- testTransitions(oid, Unapproved, Inactive)
} yield ()
}
Expand All @@ -148,7 +132,7 @@ class setObservationWorkflowState
oid <- createGmosNorthLongSlitObservationAs(pi, pid, List(tid))
_ <- createConfigurationRequestAs(pi, oid).flatMap(approveConfigurationRequest)
_ <- computeItcResult(oid)
_ <- assertIO(queryObservationWorkflowState(oid), Defined)
_ <- assertIO(queryObservationWorkflowState(oid), Defined)
_ <- testTransitions(oid, Defined, Inactive)
} yield ()

Expand All @@ -164,7 +148,7 @@ class setObservationWorkflowState
oid <- createGmosNorthLongSlitObservationAs(pi, pid, List(tid))
_ <- createConfigurationRequestAs(pi, oid).flatMap(approveConfigurationRequest)
_ <- computeItcResult(oid)
_ <- assertIO(queryObservationWorkflowState(oid), Defined)
_ <- assertIO(queryObservationWorkflowState(oid), Defined)
_ <- testTransitions(oid, Defined, Inactive, Ready)
} yield ()

Expand All @@ -182,7 +166,7 @@ class setObservationWorkflowState
_ <- addEndStepEvent(s1)
s2 <- recordStepAs(serviceUser, a, Instrument.GmosNorth, gmosNorthScience(0), StepConfig.Science, telescopeConfig(0, 0, StepGuideState.Enabled), ObserveClass.Science)
_ <- addEndStepEvent(s2)
_ <- assertIO(queryObservationWorkflowState(o), Ongoing)
_ <- assertIO(queryObservationWorkflowState(o), Ongoing)
_ <- testTransitions(o, Ongoing, Inactive)
yield ()

Expand All @@ -202,7 +186,7 @@ class setObservationWorkflowState
_ <- addEndStepEvent(s2)
s3 <- recordStepAs(serviceUser, a, Instrument.GmosNorth, gmosNorthScience(0), StepConfig.Science, telescopeConfig(0, 0, StepGuideState.Enabled), ObserveClass.Science)
_ <- addEndStepEvent(s3)
_ <- assertIO(queryObservationWorkflowState(o), Completed)
_ <- assertIO(queryObservationWorkflowState(o), Completed)
_ <- testTransitions(o, Completed)
yield ()

Expand Down

0 comments on commit e98b4f0

Please sign in to comment.