# Update StrayLightData to use FitsGenericFormatter with a deferred data set

XMLWordPrintable

#### Details

• Type: Story
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
None
• Story Points:
1
• Team:
Data Release Production
• Urgent?:
No

#### Description

The StrayLightData dataset was special cased to continue to read from a file in gen3 previously, and that will likely cause caching problems.  This can be avoided by making that dataset less of a special case, using the FitsGenericFormatter and a to-be-written readFits method.

I think this can be done in such a way that is transparent to gen2 butlers, but should be tested to confirm that gen2 behavior is unchanged.

#### Activity

Hide
Christopher Waters added a comment -

Running an example exposure in both gen2 and gen3 (as well as a gen2 control test using the default code build) yields identical images.

Final jenkins:

https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/34815/pipeline

Show
Christopher Waters added a comment - Running an example exposure in both gen2 and gen3 (as well as a gen2 control test using the default code build) yields identical images. Final jenkins: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/34815/pipeline
Hide
Nate Lust added a comment -

There is no PR for daf_butler, that will need to be created before merging I think, but I reviewed the change and it looks fine. I did wonder about additional memory usage in gen2. Before the image would not be loaded until evaluate was done, but now it is done in readIsrData. However its possible that in ISRTask that self.strayLight.check(ccdExposure) evaluates to False, and StrayLightTask.run is never called, meaning the data is read in needlessly. I am not sure how much we really need to cater to gen2 processing to worry about this though.

Show
Nate Lust added a comment - There is no PR for daf_butler, that will need to be created before merging I think, but I reviewed the change and it looks fine. I did wonder about additional memory usage in gen2. Before the image would not be loaded until evaluate was done, but now it is done in readIsrData. However its possible that in ISRTask that self.strayLight.check(ccdExposure) evaluates to False, and StrayLightTask.run is never called, meaning the data is read in needlessly. I am not sure how much we really need to cater to gen2 processing to worry about this though.
Hide
Christopher Waters added a comment -

I had originally thought the daf_butler change was needed, but as we only have straylight correction data for obs_subaru, I'm not sure it's useful to define a formatter when the only one we use is an exception to that definition.

The gen2 behavior should be correct: SubaruStrayLightTask.readIsrData does the check and returns None if it's False and skips the load.  This does move the load earlier in the program, but that didn't seem important.

Show
Christopher Waters added a comment - I had originally thought the daf_butler change was needed, but as we only have straylight correction data for obs_subaru , I'm not sure it's useful to define a formatter when the only one we use is an exception to that definition. The gen2 behavior should be correct: SubaruStrayLightTask.readIsrData does the check and returns None if it's False and skips the load.  This does move the load earlier in the program, but that didn't seem important.
Hide
Tim Jenness added a comment -

Replying to Nate Lust, the time has passed for us to be worrying about optimal gen2 usage. In the short term the only thing that matters is that we haven't completely broken gen2 with this change. In 3 months it will be moot.

Show
Tim Jenness added a comment - Replying to Nate Lust , the time has passed for us to be worrying about optimal gen2 usage. In the short term the only thing that matters is that we haven't completely broken gen2 with this change. In 3 months it will be moot.

#### People

Assignee:
Christopher Waters
Reporter:
Christopher Waters
Reviewers:
Nate Lust
Watchers:
Christopher Waters, Nate Lust, Paul Price, Tim Jenness