Keck-DataReductionPipelines / MosfireDRP

http://keck-datareductionpipelines.github.io/MosfireDRP
10 stars 13 forks source link

Flats.py fails to handle lamps off flats correctly #137

Open joshwalawender opened 5 years ago

joshwalawender commented 5 years ago

The handle_flats method in Flats.py does not appear to handle lamps off flats correctly.

First, looking at the code, the file which contains the combined lamps off data is actually formed from the lamps on data. Line 132 reads"

IO.imcombine(flatlist, out, options, reject="minmax", nlow=1, nhigh=1)

but it should presumably be:

IO.imcombine(lampOffList, out, options, reject="minmax", nlow=1, nhigh=1)

As a result of this, the difference file between the lamps on and lamps off data is all zeros (i.e. combflat_lamps_on_2d_K.fits).

Even if this were fixed and the correct data was present in combflat_lamps_on_2d_K.fits, the make_pixel_flat method which would be called with lampsOff=True does not use that flag. It does nothing with the lamps off data.

themiyan commented 5 years ago

Hi @joshwalawender has there been any updates on this? We've been reducing some K band data and the combined flats comes out as 0 because of this issue.

lucarizzi commented 5 years ago

@themiyan would you be available to work with us on this? unfortunately, we have very little bandwidth to work on MOSFIRE pipeline right now. The Flats.py portion of the pipeline contains this comment: 2014-01-09: MK - Functions added to subtract dome "lamps off/thermal" flats from the dome "lamps on" flats. The functions combine both sets of flats using your current method to create a lamps on and lamps off flat, and then subtracts those two images to remove the contribution of the dome emission. Files renamed combflat_lamps_on and combflat_lamps_off. The final flat has the same name that you output: combflat_2sband.fits . To reduce the thermal flats, the flat functions have an optional keyword for the "lampOffList" that acts as the reduction trigger. The driver file should include a call like the example below. Flats.handle_flats('Flat.txt', maskname, band, flatops, lampOffList='FlatThermal.txt')

Does this work?

joshwalawender commented 5 years ago

@themiyan @lucarizzi Sorry for being slow on the response to this. When I originally logged this, I tried to work out the code path and as far as I can tell, it is not fully implemented. I don't think handling of lamps off flats was ever completed, so it would require developing the rest of that code path.

csteidel commented 5 years ago

I think this part was worked on by Marc Kassis a while back- could the code have gotten lost in the github repository and possibly not carried forward? I seem to recall a time when this DID work...

On Apr 16, 2019, at 2:12 PM, Josh Walawender notifications@github.com wrote:

@themiyan @lucarizzi Sorry for being slow on the response to this. When I originally logged this, I tried to work out the code path and as far as I can tell, it is not fully implemented. I don't think handling of lamps off flats was ever completed, so it would require developing the rest of that code path.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

lucarizzi commented 5 years ago

The previous version of the pipeline is still here, in a repository called https://github.com/Keck-DataReductionPipelines/MosfireDRP_preWMKO so we can check.

themiyan commented 5 years ago

I agree with @csteidel I do recall it used to work at some point in the past. @lucarizzi I'll have a look at this.

lucarizzi commented 5 years ago

Is it possible that it was just a typo in Flats.py?

see commit 4669b846821a2d259647160e50985329d88a3a91

I tried on a data set and it correctly calculates the on and off flats and subtracts them.

themiyan commented 5 years ago

yes, that seems to do the trick. Sorry i didn't get a chance to look into this.

make_pixel_flat() also seems to take the lampoff list but not do anything with it.

lucarizzi commented 5 years ago

I think that make_pixel_flat() uses whatever flat you save on disk. But yes I don't think that we are actually using the on minus off flat to feed make_pixel_flat. It should be a simple fix.