Details
-
Type:
Story
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Component/s: pipe_tasks
-
Labels:None
-
Story Points:6
-
Epic Link:
-
Sprint:AP F21-6 (November), AP S22-1 (December)
-
Team:Alert Production
-
Urgent?:No
Description
ProcessCcdWithFakes and MatchFakesTask currently have dimensions of [skymap, tract, instrument, visit, detector]. This causes processing problems for data multi-tract data as the inputs with tract are ambiguous and a quantum graph can not be made.
After chatting with Jim Bosch and Sophie Reed, here's the solution to be worked on in this ticket:
- Remove tract from the two tasks dimensions (both tasks output as [instrument, visit, detector]).
- Make any tract dependent input use the multi and deferLoad options.
- Write code to decide which tract to use based on which tract the detector visit is nearest. For the fakes catalog, write code to concatenate the fakes catalog together without duplicating data.
Attachments
Issue Links
Activity
Hey Lee, let me know what ya think and feel free to loop in Sophie.
Hey Chris, I've left some comments on your various PRs. All in all, this looks to be in pretty good shape. Other than Jenkins, have you ran this through on some test data to confirm that the results are consistent with prior outputs?
I note that Surhud More also left some good comments on deprecated names and a typo, which would also be useful to look at.
I am concerned about renaming the primary synthetic source injection task label in obs_subaru and ci_hsc_gen3 from processCcdWithFakes to detectorVisitFakes. The new name, whilst consistent with ap_verify, is not in keeping with the current DRP naming convention, nor is it particularly descriptive. My suggestion, if this label must be changed, is to change it to something like singleFrameWithFakes. It would be good to get Sophie Reed's thoughts on this too.
I agree with Lee about naming, detectorVisitFakes doesn't really give me any indication of what is happening or a hint that processing beyond just addition of fakes is going on. I would second Lee's suggestion.
Thanks again Chris for the edits made on GitHub. Assuming test runs of these scripts have proceeded through without issue, then I think this looks good to merge to me - very nicely done!
Jenkins: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/35488/pipeline