Noise map inputs must be 1024x1024, not 1200x2200. Fix needed immediately

[ajeldorado]

Describe the bug corgidrp.darks.build_synthesized_dark() errors when given 1024x1024 arrays in the noisemap stack. TypeError: F must be 1200x2200

To Reproduce Try running corgidrp.darks.build_synthesized_dark() with 1024x1024 input noise maps.

Expected behavior corgidrp.darks.build_synthesized_dark() needs to have the same basic functionality of the deprecated function it replaces, cal.masterdark.gsw_masterdark.build_dark(). The input maps for build_dark() were and should all be 1024x1024 arrays because that is what all the use cases need in FSW and GSW.

corgidrp version: v1.0

Additional context This needs to be fixed immediately and included in a bug fix release (v1.0.1 ?) for GSW R3.0.1 testing.

I recommend correcting the array size checks in corgidrp.darks.build_synthesized_dark(), changing corgidrp.data.DetectorNoiseMaps to use input 1024x1024 maps (and removing the full_frame option), and then fixing anywhere else that breaks from that change.

[semaphoreP]

@ajeldorado Why does the input need to be 1024x1024? Shouldn't we only care that the output is 1024x1024? Is there a requirement that the noise maps are 1024x1024

[ajeldorado]

• The main reason is that the F, D, and C frames are also meant to be used in FSW, which can accept only 1024x1024 frames for each of them.
  • While it could be different for GSW and FSW, then we have to bookkeep two different versions which is problematic.
• A 1200x2200 master dark isn't usable for anything because L2a images already have to be 1024x1024.
• Dark current noise in the overscan is not sensible.

[ajeldorado]

I'll counter with asking, why do the inputs need to be the current 1200x2200? They are processed data products anyway and don't need to be the same size as the frames they were created from, the info outside the 1024x1024 image area is never used, and the agreement was to port over the functionality of the existing code that takes 1024x1024 inputs and creates a 1024x1024 output.

[kjl0025]

The II&T code for making noise maps (calibrate_darks_lsq) made a full-frame master dark, and this was because the EM gain measurement code (EM_gain_fit from cal repo) made use of the full-frame master dark (and the function enforces 0 dark current in non-image areas).

Could we compromise and save the calibration noise map files with a 1024x1024 HDU and another full-frame HDU?

[semaphoreP]

I'll note that we can use keywords in corgidrp.darks.build_synthesized_dark() to input 1024x1024 noise maps without crashing.

It sounds like we do need the full frame 1200x2200 noise maps for DRP calibrations. Can you just crop down the 1200x2200 into 1024x1024 as part of reading in the DRP-produced FITS files?

[maxwellmb]

I'll say that we've gotten no direction or requirements from the data formats for FSW. I'd imagine we might want different recipes for FSW products and that might resolve this too, no? e.g. with different detection_regions.

On Tue, Oct 22, 2024 at 1:50 PM Jason Wang @.***> wrote:

I'll note that we can use keywords in corgidrp.darks.build_synthesized_dark() to input 1024x1024 noise maps without crashing.

It sounds like we do need the full frame 1200x2200 noise maps for DRP calibrations. Can you just crop down the 1200x2200 into 1024x1024 as part of reading in the DRP-produced FITS files?

— Reply to this email directly, view it on GitHub https://github.com/roman-corgi/corgidrp/issues/223#issuecomment-2430239010, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEK2SJG6QWBKD7XNV7RAW63Z423CHAVCNFSM6AAAAABQNIM2RKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMZQGIZTSMBRGA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

[semaphoreP]

Yes, we can certainly produce 1024x1024 noise maps by just changing the input recipe into the DRP

[ecady-jpl]

Give me a minute and I can talk to FSW needs...

[ecady-jpl]

OK, FSW will need the following 5 things at 1024x1024 (across everything, not just darks):

• FPN array
• CIC array
• dark current noise array
• flat field array
• bad pixel array

[ecady-jpl]

Flat field is new, clearly. Bad pixels and 3x synthetic dark components were moved over from WFSC.

If there is a path to getting those 5, I think we can make this work.

[maxwellmb]

Two questions:

• Are there other general FSW requirements that we should know about?
• Are these new requirements necessary for acceptance of R3.0.1?

On Tue, Oct 22, 2024 at 2:03 PM Eric Cady @.***> wrote:

Flat field is new, clearly. Bad pixels and 3x synthetic dark components were moved over from WFSC.

If there is a path to getting those 5, I think we can make this work.

— Reply to this email directly, view it on GitHub https://github.com/roman-corgi/corgidrp/issues/223#issuecomment-2430276179, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEK2SJE2LHBSTJ42OBUDUG3Z424R5AVCNFSM6AAAAABQNIM2RKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMZQGI3TMMJXHE . You are receiving this because you commented.Message ID: @.***>

[ecady-jpl]

And to address @ajeldorado's original concern: the standing VAP functional tests load in FSW-sized arrays. Is there a native way in corgidrp to embed an image-sized array in a full frame?

We can throw something together with tools from another repo, or rebuild the test data maybe, but it would be preferred to use corgidrp tools if available.

[ecady-jpl]

@maxwellmb

• This is a requirement in that FSW has allocated a memory region for each of these and the data needs to be the right size for the buffer, and it won't be allowed to be uploaded if it's the wrong size. There is a very thin L1 -> L2a -> L2b pipeline onboard and it requires a subset of the products used in corgidrp.
• There are also a couple of scalars that will need to be extracted from tools now in corgidrp: K-gain and bias offset
• As long as there is a path to producing these products with your tools, we should not have to hold up R3.0.1.

[maxwellmb]

Eric, thanks for the reply. I'm also more generally wondering if there are formal requirements written down anywhere for what the FSW needs from the DRP just to make sure we don't miss anything else and can incorporate it into future tests.

There's a chance there will be some debugging, but there's nothing fundamental about the DRP that wouldn't allow this. In fact using the detector_regions keyword we can probably resolve this without changing code, and only changing a recipe.

On Tue, Oct 22, 2024 at 2:23 PM Eric Cady @.***> wrote:

@maxwellmb https://github.com/maxwellmb

• This is a requirement in that FSW has allocated a memory region for each of these and the data needs to be the right size for the buffer, and it won't be allowed to be uploaded if it's the wrong size. There is a very thin L1 -> L2a -> L2b pipeline onboard and it requires a subset of the products used in corgidrp.
• There are also a couple of scalars that will need to be extracted from tools now in corgidrp: K-gain and bias offset
• As long as there is a path to producing these products with your tools, we should not have to hold up R3.0.1.

— Reply to this email directly, view it on GitHub https://github.com/roman-corgi/corgidrp/issues/223#issuecomment-2430337397, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEK2SJGLFPSSFFOZO5PE7GLZ4265PAVCNFSM6AAAAABQNIM2RKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMZQGMZTOMZZG4 . You are receiving this because you were mentioned.Message ID: @.***>

[ajeldorado]

That brings up another issue...should I make a formal GitHub Issue for it? We can't directly access things in the docker image containing corgidrp that are data types, only functions, classes, and methods. So that means we can't just load corgidrp.detector.detector_areas because it is a dictionary--it would need a very thin function wrapper that returns the dictionary for us to access it. There may be other cases like this in the other modules too. Note: This isn't a deal breaker for R3.0.1, but I would like it to change next time so I don't have to copy-paste stuff from inside corgidrp into the test scripts calling the docker image of it.

[semaphoreP]

Ok, so it sounds like we should be outputting 1024x1024 noise maps for FSW. From my reading of the discussion above, it's ok if we have to alter the default parameters and use a specialized recipe to produce these 1024x1024 noise maps. Please confirm this is ok, and we will write a test that checks this functionality works.

I believe flat field and bad pixel maps are already 1024x1024 by default, so this is the only one that needs adjusted.

@ajeldorado I don't understand why you can't access corgidrp.detector.detector_areas. Could you give some more details? In python, everything is an object (including functions/classes/methods), so you can do import corgidrp.detector; print(corgidrp.detector.detector_areas). And docker itself is fairly agnostic to exact python behavior. So presumably there is some intermediate interface you are dealing with? If so, we're happy to write a wrapper function, but I want to understand the limitations here.