Simplify and unify ITC model #4270
Conversation
BundleMonFiles updated (2)
Unchanged files (6)
Total files change +868B +0.03% Final result: ✅ View report in BundleMon website ➡️ |
| ).some | ||
| ) | ||
| >> props.deleteConfig | ||
| onClick = props.deleteConfig |
There was a problem hiding this comment.
Should this be revertConfig?
| yield ItcClient[IO] | ||
| .requestSingle: | ||
| ItcMessage.GraphQuery(w, sn, snAt, constraints, t, mode) | ||
| .map: | ||
| _.toRight(new Throwable("No response from ITC server.")) | ||
| .rethrow | ||
| .flatTap: result => |
| .useState(Pot.pending[ItcAsterismGraphResults]) // itcGraphResults | ||
| .useAsyncEffectWithDepsBy((props, _, _, _, selectedConfig, _, _) => | ||
| itcQueryProps(props.observation.get, selectedConfig.get, props.allTargets) | ||
| ItcProps(props.observation.get, selectedConfig.get, props.obsTargets) |
There was a problem hiding this comment.
Caan this be combined in one hook? Like one of those useResourceWhen...
There was a problem hiding this comment.
Let me check. At the very least, we can use a localVal so that we don't build ItcProps twice.
There was a problem hiding this comment.
I was able to simplify quite a bit by using localVal and useEffectResult. Thanks for the suggestion!
| InstrumentOverrides | ||
| .GmosSpectroscopy( | ||
| cw, | ||
| GmosCcdMode.defaultGmosNorth(profiles, fpu, grating, imageQuality), |
There was a problem hiding this comment.
Nice, I didn't notice this method. Is it from lucuma-core?
There was a problem hiding this comment.
It's an extension I created. But I think it should be moved to lucuma-core in the future.
| import lucuma.core.model.sequence.gmos.longslit.* | ||
|
|
||
| object syntax: | ||
| // TODO Actually move to lucuma-core, to GmosCcdMode |
| none | ||
|
|
||
| def doRequest(request: ItcGraphRequestParams): F[ItcAsterismGraphResults] = | ||
| request.mode.toItcClientMode | ||
| .map: mode => | ||
| println(s"request for mode $mode") |
| @@ -73,7 +71,7 @@ object ITCRequests: | |||
| params: ItcRequestParams | |||
| ): F[Option[Map[ItcRequestParams, EitherNec[ItcTargetProblem, ItcResult]]]] = | |||
| Logger[F] | |||
| .debug( | |||
| .info( | |||
There was a problem hiding this comment.
Should it go back to debug?
rpiaggio
deleted the
sc-3813-different-itc-output-before-and-after-accepting
branch
A bug was reported where invocations to ITC did not include CCD mode information when invoked before selecting a configuration.
This PR attempts to fix that by computing the necessary overrides for the current observations. This is now computed and stored in
InstrumentConfig(previously known asInstrumentRow) when computing each possible row of the modes table. Also, the selected configuration is now onInstrumentConfiginstead of aBasicConfig. This simplifies keeping track of the overrides since now they are in the selected object. In particular,ItcPropsis simplified.ITC results before and after configuration selection are now consistent.