From 562686129093e3d972f9f87a050f834d932413b9 Mon Sep 17 00:00:00 2001 From: jhennawi Date: Sat, 1 Jun 2024 17:34:13 -0700 Subject: [PATCH 01/14] refactoring pypeit image --- pypeit/images/buildimage.py | 27 +++++--- pypeit/images/combineimage.py | 115 ++++++++++++---------------------- 2 files changed, 60 insertions(+), 82 deletions(-) diff --git a/pypeit/images/buildimage.py b/pypeit/images/buildimage.py index b9ecfa9339..38fd2f6dea 100644 --- a/pypeit/images/buildimage.py +++ b/pypeit/images/buildimage.py @@ -10,11 +10,13 @@ from pypeit import msgs from pypeit.par import pypeitpar +from pypeit.images import rawimage from pypeit.images import combineimage from pypeit.images import pypeitimage from pypeit.core.framematch import valid_frametype + class ArcImage(pypeitimage.PypeItCalibrationImage): """ Simple DataContainer for the Arc Image @@ -160,7 +162,10 @@ def buildimage_fromlist(spectrograph, det, frame_par, file_list, bias=None, bpm= scattlight=None, flatimages=None, maxiters=5, ignore_saturation=True, slits=None, mosaic=None, calib_dir=None, setup=None, calib_id=None): """ - Perform basic image processing on a list of images and combine the results. + Perform basic image processing on a list of images and combine the results. All + core processing steps for each image are handled by :class:`~pypeit.images.rawimage.RawImage` and + image combination is handled by :class:`~pypeit.images.combineimage.CombineImage`. + This function can be used to process both single images, lists of images, and detector mosaics. .. warning:: @@ -246,15 +251,23 @@ def buildimage_fromlist(spectrograph, det, frame_par, file_list, bias=None, bpm= # Should the detectors be reformatted into a single image mosaic? if mosaic is None: mosaic = isinstance(det, tuple) and frame_par['frametype'] not in ['bias', 'dark'] - + + rawImage_list = [] + # Loop on the files + for ifile in file_list: + # Load raw image + rawImage = rawimage.RawImage(ifile, spectrograph, det) + # Process + rawImage_list.append(rawImage.process( + frame_par['process'], scattlight=scattlight, bias=bias, + bpm=bpm, dark=dark, flatimages=flatimages, slits=slits, mosaic=mosaic)) + # Do it - combineImage = combineimage.CombineImage(spectrograph, det, frame_par['process'], file_list) - pypeitImage = combineImage.run(bias=bias, bpm=bpm, dark=dark, flatimages=flatimages, scattlight=scattlight, - sigma_clip=frame_par['process']['clip'], + combineImage = combineimage.CombineImage(rawImage_list, frame_par['process']) + pypeitImage = combineImage.run(sigma_clip=frame_par['process']['clip'], sigrej=frame_par['process']['comb_sigrej'], maxiters=maxiters, ignore_saturation=ignore_saturation, - slits=slits, combine_method=frame_par['process']['combine'], - mosaic=mosaic) + combine_method=frame_par['process']['combine']) # Return class type, if returning any of the frame_image_classes cls = frame_image_classes[frame_par['frametype']] \ diff --git a/pypeit/images/combineimage.py b/pypeit/images/combineimage.py index ffb0b20247..70596822a5 100644 --- a/pypeit/images/combineimage.py +++ b/pypeit/images/combineimage.py @@ -16,28 +16,19 @@ from pypeit.par import pypeitpar from pypeit import utils from pypeit.images import pypeitimage -from pypeit.images import rawimage from pypeit.images import imagebitmask class CombineImage: """ - Process and combine detector images. - - All core processing steps for each image are handled by - :class:`~pypeit.images.rawimage.RawImage`. This class can be used to - process both single images, lists of images, and detector mosaics. + Process and combine detector images. Args: - spectrograph (:class:`~pypeit.spectrographs.spectrograph.Spectrograph`): - Spectrograph used to take the data. - det (:obj:`int`, :obj:`tuple`): - The 1-indexed detector number(s) to process. If a tuple, it must - include detectors viable as a mosaic for the provided spectrograph; - see :func:`~pypeit.spectrographs.spectrograph.Spectrograph.allowed_mosaics`. + Parameters that dictate the processing of the images. + rawImages (:obj:`list`): + Either a single :class:`~pypeit.images.rawimage.RawImage` object or a list of one or more + of these objects to be combined into a an image. par (:class:`~pypeit.par.pypeitpar.ProcessImagesPar`): Parameters that dictate the processing of the images. - files (:obj:`str`, array-like): - A set of one or more images to process and combine. Attributes: spectrograph (:class:`~pypeit.spectrographs.spectrograph.Spectrograph`): @@ -46,24 +37,22 @@ class CombineImage: The 1-indexed detector number(s) to process. par (:class:`~pypeit.par.pypeitpar.ProcessImagesPar`): Parameters that dictate the processing of the images. - files (:obj:`list`): - A set of one or more images to process and combine. + rawImages (:obj:`list`): + A list of one or more :class:`~pypeit.images.rawimage.RawImage` objects + to be combined. """ - def __init__(self, spectrograph, det, par, files): - self.spectrograph = spectrograph - self.det = det + def __init__(self, rawImages, par): if not isinstance(par, pypeitpar.ProcessImagesPar): msgs.error('Provided ParSet for must be type ProcessImagesPar.') self.par = par # This musts be named this way as it is frequently a child - self.files = list(files) if hasattr(files, '__len__') else [files] - # NOTE: nfiles is a property method. Defining files above must come + self.rawImages = list(rawImages) if hasattr(rawImages, '__len__') else [rawImages] + # NOTE: nimgs is a property method. Defining rawImages above must come # before this check! - if self.nfiles == 0: + if self.nimgs == 0: msgs.error('CombineImage requires a list of files to instantiate') - def run(self, bias=None, scattlight=None, flatimages=None, ignore_saturation=False, sigma_clip=True, - bpm=None, sigrej=None, maxiters=5, slits=None, dark=None, combine_method='mean', mosaic=False): + def run(self, ignore_saturation=False, sigma_clip=True, sigrej=None, maxiters=5, combine_method='mean'): r""" Process and combine all images. @@ -133,14 +122,6 @@ def run(self, bias=None, scattlight=None, flatimages=None, ignore_saturation=Fal in images of the same shape. Args: - bias (:class:`~pypeit.images.buildimage.BiasImage`, optional): - Bias image for bias subtraction; passed directly to - :func:`~pypeit.images.rawimage.RawImage.process` for all images. - scattlight (:class:`~pypeit.scattlight.ScatteredLight`, optional): - Scattered light model to be used to determine scattered light. - flatimages (:class:`~pypeit.flatfield.FlatImages`, optional): - Flat-field images for flat fielding; passed directly to - :func:`~pypeit.images.rawimage.RawImage.process` for all images. ignore_saturation (:obj:`bool`, optional): If True, turn off the saturation flag in the individual images before stacking. This avoids having such values set to 0, which @@ -149,9 +130,6 @@ def run(self, bias=None, scattlight=None, flatimages=None, ignore_saturation=Fal sigma_clip (:obj:`bool`, optional): When ``combine_method='mean'``, perform a sigma-clip the data; see :func:`~pypeit.core.combine.weighted_combine`. - bpm (`numpy.ndarray`_, optional): - Bad pixel mask; passed directly to - :func:`~pypeit.images.rawimage.RawImage.process` for all images. sigrej (:obj:`float`, optional): When ``combine_method='mean'``, this sets the sigma-rejection thresholds used when sigma-clipping the image combination. @@ -165,12 +143,6 @@ def run(self, bias=None, scattlight=None, flatimages=None, ignore_saturation=Fal rejection iterations. If None, rejection iterations continue until no more data are rejected; see :func:`~pypeit.core.combine.weighted_combine``. - slits (:class:`~pypeit.slittrace.SlitTraceSet`, optional): - Slit edge trace locations; passed directly to - :func:`~pypeit.images.rawimage.RawImage.process` for all images. - dark (:class:`~pypeit.images.buildimage.DarkImage`, optional): - Dark-current image; passed directly to - :func:`~pypeit.images.rawimage.RawImage.process` for all images. combine_method (:obj:`str`, optional): Method used to combine images. Must be ``'mean'`` or ``'median'``; see above. @@ -184,60 +156,53 @@ def run(self, bias=None, scattlight=None, flatimages=None, ignore_saturation=Fal all the processed images. """ # Check the input (i.e., bomb out *before* it does any processing) - if self.nfiles == 0: + if self.nimgs == 0: msgs.error('Object contains no files to process!') - if self.nfiles > 1 and combine_method not in ['mean', 'median']: + if self.nimgs > 1 and combine_method not in ['mean', 'median']: msgs.error(f'Unknown image combination method, {combine_method}. Must be ' '"mean" or "median".') # Loop on the files - for kk, ifile in enumerate(self.files): - # Load raw image - rawImage = rawimage.RawImage(ifile, self.spectrograph, self.det) - # Process - pypeitImage = rawImage.process(self.par, scattlight=scattlight, bias=bias, bpm=bpm, dark=dark, - flatimages=flatimages, slits=slits, mosaic=mosaic) - - if self.nfiles == 1: + for kk, rawImage in enumerate(self.rawImages): + if self.nimgs == 1: # Only 1 file, so we're done - pypeitImage.files = self.files - return pypeitImage + return rawImage elif kk == 0: # Allocate arrays to collect data for each frame - shape = (self.nfiles,) + pypeitImage.shape + shape = (self.nimgs,) + rawImage.shape img_stack = np.zeros(shape, dtype=float) scl_stack = np.ones(shape, dtype=float) rn2img_stack = np.zeros(shape, dtype=float) basev_stack = np.zeros(shape, dtype=float) gpm_stack = np.zeros(shape, dtype=bool) - lampstat = [None]*self.nfiles - exptime = np.zeros(self.nfiles, dtype=float) + lampstat = [None]*self.nimgs + exptime = np.zeros(self.nimgs, dtype=float) # Save the lamp status # TODO: As far as I can tell, this is the *only* place rawheadlist # is used. Is there a way we can get this from fitstbl instead? - lampstat[kk] = self.spectrograph.get_lamps_status(pypeitImage.rawheadlist) + lampstat[kk] = rawImage.spectrograph.get_lamps_status(rawImage.rawheadlist) # Save the exposure time to check if it's consistent for all images. - exptime[kk] = pypeitImage.exptime + exptime[kk] = rawImage.exptime # Processed image - img_stack[kk] = pypeitImage.image + img_stack[kk] = rawImage.image # Get the count scaling - if pypeitImage.img_scale is not None: - scl_stack[kk] = pypeitImage.img_scale + if rawImage.img_scale is not None: + scl_stack[kk] = rawImage.img_scale # Read noise squared image - if pypeitImage.rn2img is not None: - rn2img_stack[kk] = pypeitImage.rn2img * scl_stack[kk]**2 + if rawImage.rn2img is not None: + rn2img_stack[kk] = rawImage.rn2img * scl_stack[kk]**2 # Processing variance image - if pypeitImage.base_var is not None: - basev_stack[kk] = pypeitImage.base_var * scl_stack[kk]**2 + if rawImage.base_var is not None: + basev_stack[kk] = rawImage.base_var * scl_stack[kk]**2 # Final mask for this image # TODO: This seems kludgy to me. Why not just pass ignore_saturation # to process_one and ignore the saturation when the mask is actually # built, rather than untoggling the bit here? if ignore_saturation: # Important for calibrations as we don't want replacement by 0 - pypeitImage.update_mask('SATURATION', action='turn_off') + rawImage.update_mask('SATURATION', action='turn_off') # Get a simple boolean good-pixel mask for all the unmasked pixels - gpm_stack[kk] = pypeitImage.select_flag(invert=True) + gpm_stack[kk] = rawImage.select_flag(invert=True) # Check that the lamps being combined are all the same: if not lampstat[1:] == lampstat[:-1]: @@ -266,7 +231,7 @@ def run(self, bias=None, scattlight=None, flatimages=None, ignore_saturation=Fal # Coadd them if combine_method == 'mean': - weights = np.ones(self.nfiles, dtype=float)/self.nfiles + weights = np.ones(self.nimgs, dtype=float)/self.nimgs img_list_out, var_list_out, gpm, nframes \ = combine.weighted_combine(weights, [img_stack, scl_stack], # images to stack @@ -310,14 +275,14 @@ def run(self, bias=None, scattlight=None, flatimages=None, ignore_saturation=Fal # Build the combined image comb = pypeitimage.PypeItImage(image=comb_img, ivar=utils.inverse(comb_var), nimg=nframes, - amp_img=pypeitImage.amp_img, det_img=pypeitImage.det_img, + amp_img=rawImage.amp_img, det_img=rawImage.det_img, rn2img=comb_rn2, base_var=comb_basev, img_scale=comb_scl, # NOTE: This *must* be a boolean. bpm=np.logical_not(gpm), # NOTE: The detector is needed here so # that we can get the dark current later. - detector=pypeitImage.detector, - PYP_SPEC=self.spectrograph.name, + detector=rawImage.detector, + PYP_SPEC=rawImage.spectrograph.name, units='e-' if self.par['apply_gain'] else 'ADU', exptime=comb_texp, noise_floor=self.par['noise_floor'], shot_noise=self.par['shot_noise']) @@ -325,8 +290,8 @@ def run(self, bias=None, scattlight=None, flatimages=None, ignore_saturation=Fal # Internals # TODO: Do we need these? comb.files = self.files - comb.rawheadlist = pypeitImage.rawheadlist - comb.process_steps = pypeitImage.process_steps + comb.rawheadlist = rawImage.rawheadlist + comb.process_steps = rawImage.process_steps # Build the base level mask comb.build_mask(saturation='default', mincounts='default') @@ -338,10 +303,10 @@ def run(self, bias=None, scattlight=None, flatimages=None, ignore_saturation=Fal return comb @property - def nfiles(self): + def nimgs(self): """ The number of files in :attr:`files`. """ - return len(self.files) if isinstance(self.files, (np.ndarray, list)) else 0 + return len(self.rawImages) if isinstance(self.rawImages, (np.ndarray, list)) else 0 From 383ac4845850a16608632a8e3d50d1ba9d4ccef2 Mon Sep 17 00:00:00 2001 From: jhennawi Date: Sat, 1 Jun 2024 17:41:23 -0700 Subject: [PATCH 02/14] new --- pypeit/images/buildimage.py | 3 ++- pypeit/images/combineimage.py | 14 +++++++++----- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/pypeit/images/buildimage.py b/pypeit/images/buildimage.py index 38fd2f6dea..1b74f63eb3 100644 --- a/pypeit/images/buildimage.py +++ b/pypeit/images/buildimage.py @@ -263,11 +263,12 @@ def buildimage_fromlist(spectrograph, det, frame_par, file_list, bias=None, bpm= bpm=bpm, dark=dark, flatimages=flatimages, slits=slits, mosaic=mosaic)) # Do it - combineImage = combineimage.CombineImage(rawImage_list, frame_par['process']) + combineImage = combineimage.CombineImage(rawImage_list, spectrograph, frame_par['process']) pypeitImage = combineImage.run(sigma_clip=frame_par['process']['clip'], sigrej=frame_par['process']['comb_sigrej'], maxiters=maxiters, ignore_saturation=ignore_saturation, combine_method=frame_par['process']['combine']) + combineImage.files = file_list # Return class type, if returning any of the frame_image_classes cls = frame_image_classes[frame_par['frametype']] \ diff --git a/pypeit/images/combineimage.py b/pypeit/images/combineimage.py index 70596822a5..db50057ea9 100644 --- a/pypeit/images/combineimage.py +++ b/pypeit/images/combineimage.py @@ -27,6 +27,8 @@ class CombineImage: rawImages (:obj:`list`): Either a single :class:`~pypeit.images.rawimage.RawImage` object or a list of one or more of these objects to be combined into a an image. + spectrograph (:class:`~pypeit.spectrographs.spectrograph.Spectrograph`): + Spectrograph used to take the data. par (:class:`~pypeit.par.pypeitpar.ProcessImagesPar`): Parameters that dictate the processing of the images. @@ -42,11 +44,13 @@ class CombineImage: to be combined. """ - def __init__(self, rawImages, par): + def __init__(self, rawImages, spectrograph, par): if not isinstance(par, pypeitpar.ProcessImagesPar): msgs.error('Provided ParSet for must be type ProcessImagesPar.') - self.par = par # This musts be named this way as it is frequently a child self.rawImages = list(rawImages) if hasattr(rawImages, '__len__') else [rawImages] + self.spectrograph = spectrograph + self.par = par # This musts be named this way as it is frequently a child + # NOTE: nimgs is a property method. Defining rawImages above must come # before this check! if self.nimgs == 0: @@ -181,7 +185,7 @@ def run(self, ignore_saturation=False, sigma_clip=True, sigrej=None, maxiters=5, # Save the lamp status # TODO: As far as I can tell, this is the *only* place rawheadlist # is used. Is there a way we can get this from fitstbl instead? - lampstat[kk] = rawImage.spectrograph.get_lamps_status(rawImage.rawheadlist) + lampstat[kk] = self.spectrograph.get_lamps_status(rawImage.rawheadlist) # Save the exposure time to check if it's consistent for all images. exptime[kk] = rawImage.exptime # Processed image @@ -282,14 +286,14 @@ def run(self, ignore_saturation=False, sigma_clip=True, sigrej=None, maxiters=5, # NOTE: The detector is needed here so # that we can get the dark current later. detector=rawImage.detector, - PYP_SPEC=rawImage.spectrograph.name, + PYP_SPEC=self.spectrograph.name, units='e-' if self.par['apply_gain'] else 'ADU', exptime=comb_texp, noise_floor=self.par['noise_floor'], shot_noise=self.par['shot_noise']) # Internals # TODO: Do we need these? - comb.files = self.files + #comb.files = self.files comb.rawheadlist = rawImage.rawheadlist comb.process_steps = rawImage.process_steps From db7634398c38873f5ebbfc653eca15061b03dad5 Mon Sep 17 00:00:00 2001 From: jhennawi Date: Sat, 1 Jun 2024 18:05:15 -0700 Subject: [PATCH 03/14] refactored image combining to work on a set of image objects --- pypeit/images/buildimage.py | 2 -- pypeit/images/combineimage.py | 6 ++++-- pypeit/images/pypeitimage.py | 7 ++++--- pypeit/images/rawimage.py | 1 + 4 files changed, 9 insertions(+), 7 deletions(-) diff --git a/pypeit/images/buildimage.py b/pypeit/images/buildimage.py index 1b74f63eb3..bbafc6f3e3 100644 --- a/pypeit/images/buildimage.py +++ b/pypeit/images/buildimage.py @@ -268,8 +268,6 @@ def buildimage_fromlist(spectrograph, det, frame_par, file_list, bias=None, bpm= sigrej=frame_par['process']['comb_sigrej'], maxiters=maxiters, ignore_saturation=ignore_saturation, combine_method=frame_par['process']['combine']) - combineImage.files = file_list - # Return class type, if returning any of the frame_image_classes cls = frame_image_classes[frame_par['frametype']] \ if frame_par['frametype'] in frame_image_classes.keys() else None diff --git a/pypeit/images/combineimage.py b/pypeit/images/combineimage.py index db50057ea9..bc9d0439f6 100644 --- a/pypeit/images/combineimage.py +++ b/pypeit/images/combineimage.py @@ -165,11 +165,12 @@ def run(self, ignore_saturation=False, sigma_clip=True, sigrej=None, maxiters=5, if self.nimgs > 1 and combine_method not in ['mean', 'median']: msgs.error(f'Unknown image combination method, {combine_method}. Must be ' '"mean" or "median".') - + file_list = [] # Loop on the files for kk, rawImage in enumerate(self.rawImages): if self.nimgs == 1: # Only 1 file, so we're done + rawImage.files = [rawImage.filename] return rawImage elif kk == 0: # Allocate arrays to collect data for each frame @@ -207,6 +208,7 @@ def run(self, ignore_saturation=False, sigma_clip=True, sigrej=None, maxiters=5, rawImage.update_mask('SATURATION', action='turn_off') # Get a simple boolean good-pixel mask for all the unmasked pixels gpm_stack[kk] = rawImage.select_flag(invert=True) + file_list.append(rawImage.filename) # Check that the lamps being combined are all the same: if not lampstat[1:] == lampstat[:-1]: @@ -293,7 +295,7 @@ def run(self, ignore_saturation=False, sigma_clip=True, sigrej=None, maxiters=5, # Internals # TODO: Do we need these? - #comb.files = self.files + comb.files = file_list comb.rawheadlist = rawImage.rawheadlist comb.process_steps = rawImage.process_steps diff --git a/pypeit/images/pypeitimage.py b/pypeit/images/pypeitimage.py index 91d8f118e0..cb9bb1a28f 100644 --- a/pypeit/images/pypeitimage.py +++ b/pypeit/images/pypeitimage.py @@ -127,7 +127,8 @@ class PypeItImage(datamodel.DataContainer): 'shot_noise': dict(otype=bool, descr='Shot-noise included in variance'), 'spat_flexure': dict(otype=float, descr='Shift, in spatial pixels, between this image ' - 'and SlitTrace')} + 'and SlitTrace'), + 'filename': dict(otype=str, descr='Filename for the image'),} """Data model components.""" internals = ['process_steps', 'files', 'rawheadlist'] @@ -162,8 +163,8 @@ def from_pypeitimage(cls, pypeitImage): def __init__(self, image, ivar=None, nimg=None, amp_img=None, det_img=None, rn2img=None, base_var=None, img_scale=None, fullmask=None, detector=None, spat_flexure=None, - PYP_SPEC=None, units=None, exptime=None, noise_floor=None, shot_noise=None, - bpm=None, crmask=None, usermask=None, clean_mask=False): + filename=None, PYP_SPEC=None, units=None, exptime=None, noise_floor=None, + shot_noise=None, bpm=None, crmask=None, usermask=None, clean_mask=False): if image is None: msgs.error('Must provide an image when instantiating PypeItImage.') diff --git a/pypeit/images/rawimage.py b/pypeit/images/rawimage.py index 8108c4b828..eb7c84324c 100644 --- a/pypeit/images/rawimage.py +++ b/pypeit/images/rawimage.py @@ -677,6 +677,7 @@ def process(self, par, bpm=None, scattlight=None, flatimages=None, bias=None, sl shot_noise=self.par['shot_noise'], bpm=_bpm.astype(bool)) + pypeitImage.filename = self.filename pypeitImage.rawheadlist = self.headarr pypeitImage.process_steps = [key for key in self.steps.keys() if self.steps[key]] From 85917f7abfff0f32971bf2e3b176029c701fad59 Mon Sep 17 00:00:00 2001 From: jhennawi Date: Sun, 2 Jun 2024 14:16:26 -0700 Subject: [PATCH 04/14] removed unncessary parameter arguments --- pypeit/images/buildimage.py | 5 +---- pypeit/images/combineimage.py | 38 +++++++++++------------------------ 2 files changed, 13 insertions(+), 30 deletions(-) diff --git a/pypeit/images/buildimage.py b/pypeit/images/buildimage.py index bbafc6f3e3..dbd18d688b 100644 --- a/pypeit/images/buildimage.py +++ b/pypeit/images/buildimage.py @@ -264,10 +264,7 @@ def buildimage_fromlist(spectrograph, det, frame_par, file_list, bias=None, bpm= # Do it combineImage = combineimage.CombineImage(rawImage_list, spectrograph, frame_par['process']) - pypeitImage = combineImage.run(sigma_clip=frame_par['process']['clip'], - sigrej=frame_par['process']['comb_sigrej'], - maxiters=maxiters, ignore_saturation=ignore_saturation, - combine_method=frame_par['process']['combine']) + pypeitImage = combineImage.run(maxiters=maxiters, ignore_saturation=ignore_saturation) # Return class type, if returning any of the frame_image_classes cls = frame_image_classes[frame_par['frametype']] \ if frame_par['frametype'] in frame_image_classes.keys() else None diff --git a/pypeit/images/combineimage.py b/pypeit/images/combineimage.py index bc9d0439f6..22687e1c44 100644 --- a/pypeit/images/combineimage.py +++ b/pypeit/images/combineimage.py @@ -56,7 +56,8 @@ def __init__(self, rawImages, spectrograph, par): if self.nimgs == 0: msgs.error('CombineImage requires a list of files to instantiate') - def run(self, ignore_saturation=False, sigma_clip=True, sigrej=None, maxiters=5, combine_method='mean'): + + def run(self, ignore_saturation=False, maxiters=5): r""" Process and combine all images. @@ -68,7 +69,7 @@ def run(self, ignore_saturation=False, sigma_clip=True, sigrej=None, maxiters=5, file and returns the result. If there are multiple files, all the files are processed and the - processed images are combined based on the ``combine_method``, where the + processed images are combined based on the ``par['combine']``, where the options are: - 'mean': If ``sigma_clip`` is True, this is a sigma-clipped mean; @@ -131,39 +132,24 @@ def run(self, ignore_saturation=False, sigma_clip=True, sigrej=None, maxiters=5, before stacking. This avoids having such values set to 0, which for certain images (e.g. flat calibrations) can have unintended consequences. - sigma_clip (:obj:`bool`, optional): - When ``combine_method='mean'``, perform a sigma-clip the data; - see :func:`~pypeit.core.combine.weighted_combine`. - sigrej (:obj:`float`, optional): - When ``combine_method='mean'``, this sets the sigma-rejection - thresholds used when sigma-clipping the image combination. - Ignored if ``sigma_clip`` is False. If None and ``sigma_clip`` - is True, the thresholds are determined automatically based on - the number of images provided; see - :func:`~pypeit.core.combine.weighted_combine``. maxiters (:obj:`int`, optional): - When ``combine_method='mean'``) and sigma-clipping + When ``par['combine']='mean'``) and sigma-clipping (``sigma_clip`` is True), this sets the maximum number of rejection iterations. If None, rejection iterations continue until no more data are rejected; see :func:`~pypeit.core.combine.weighted_combine``. - combine_method (:obj:`str`, optional): - Method used to combine images. Must be ``'mean'`` or - ``'median'``; see above. - mosaic (:obj:`bool`, optional): - If multiple detectors are being processes, mosaic them into a - single image. See - :func:`~pypeit.images.rawimage.RawImage.process`. Returns: :class:`~pypeit.images.pypeitimage.PypeItImage`: The combination of all the processed images. """ + + # Check the input (i.e., bomb out *before* it does any processing) if self.nimgs == 0: msgs.error('Object contains no files to process!') - if self.nimgs > 1 and combine_method not in ['mean', 'median']: - msgs.error(f'Unknown image combination method, {combine_method}. Must be ' + if self.nimgs > 1 and self.par['combine'] not in ['mean', 'median']: + msgs.error(f'Unknown image combination method, {self.par['combine']}. Must be ' '"mean" or "median".') file_list = [] # Loop on the files @@ -236,21 +222,21 @@ def run(self, ignore_saturation=False, sigma_clip=True, sigrej=None, maxiters=5, comb_texp = exptime[0] # Coadd them - if combine_method == 'mean': + if self.par['combine'] == 'mean': weights = np.ones(self.nimgs, dtype=float)/self.nimgs img_list_out, var_list_out, gpm, nframes \ = combine.weighted_combine(weights, [img_stack, scl_stack], # images to stack [rn2img_stack, basev_stack], # variances to stack - gpm_stack, sigma_clip=sigma_clip, + gpm_stack, sigma_clip=self.par['clip'], sigma_clip_stack=img_stack, # clipping based on img - sigrej=sigrej, maxiters=maxiters) + sigrej=self.par['comb_sigrej'], maxiters=maxiters) comb_img, comb_scl = img_list_out comb_rn2, comb_basev = var_list_out # Divide by the number of images that contributed to each pixel comb_scl[gpm] /= nframes[gpm] - elif combine_method == 'median': + elif self.par['combine'] == 'median': bpm_stack = np.logical_not(gpm_stack) nframes = np.sum(gpm_stack, axis=0) gpm = nframes > 0 From 2463c70f44963d4060efe133070b3b7c3bc7e96f Mon Sep 17 00:00:00 2001 From: jhennawi Date: Sun, 2 Jun 2024 14:53:57 -0700 Subject: [PATCH 05/14] fixed some issues with saturation masking --- pypeit/images/combineimage.py | 4 +++- pypeit/pypeit.py | 3 +++ pypeit/spectrographs/jwst_nirspec.py | 16 ++++++++++++++++ 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/pypeit/images/combineimage.py b/pypeit/images/combineimage.py index 22687e1c44..f66d327f04 100644 --- a/pypeit/images/combineimage.py +++ b/pypeit/images/combineimage.py @@ -196,6 +196,8 @@ def run(self, ignore_saturation=False, maxiters=5): gpm_stack[kk] = rawImage.select_flag(invert=True) file_list.append(rawImage.filename) + # TODO JFH: This should not be here, but rather should be done in the Calibrations class. Putting in calibration specific + # lamp checking in an image combining class is not ideal. # Check that the lamps being combined are all the same: if not lampstat[1:] == lampstat[:-1]: msgs.warn("The following files contain different lamp status") @@ -286,7 +288,7 @@ def run(self, ignore_saturation=False, maxiters=5): comb.process_steps = rawImage.process_steps # Build the base level mask - comb.build_mask(saturation='default', mincounts='default') + comb.build_mask(saturation='default' if not ignore_saturation else None, mincounts='default') # Flag all pixels with no contributions from any of the stacked images. comb.update_mask('STCKMASK', indx=np.logical_not(gpm)) diff --git a/pypeit/pypeit.py b/pypeit/pypeit.py index 389639a613..b4a48f7e62 100644 --- a/pypeit/pypeit.py +++ b/pypeit/pypeit.py @@ -789,6 +789,9 @@ def objfind_one(self, frames, det, bg_frames=None, std_outfile=None): bkg_redux_sciimg = sciImg # Build the background image bg_file_list = self.fitstbl.frame_paths(bg_frames) + # TODO I think we should create a separate self.par['bkgframe'] parameter set to hold the image + # processing parameters for the background frames. This would allow the user to specify different + # parameters for the background frames than for the science frames. bgimg = buildimage.buildimage_fromlist(self.spectrograph, det, frame_par, bg_file_list, bpm=self.caliBrate.msbpm, bias=self.caliBrate.msbias, diff --git a/pypeit/spectrographs/jwst_nirspec.py b/pypeit/spectrographs/jwst_nirspec.py index 1a5fb586e8..088670fa39 100644 --- a/pypeit/spectrographs/jwst_nirspec.py +++ b/pypeit/spectrographs/jwst_nirspec.py @@ -255,6 +255,22 @@ def allowed_mosaics(self): """ return [(1,2)] + + + def get_lamps_status(self, headarr): + """ + Return a string containing the information on the lamp status. + + Args: + headarr (:obj:`list`): + A list of 1 or more `astropy.io.fits.Header`_ objects. + + Returns: + :obj:`str`: A string that uniquely represents the lamp status. + """ + # TODO: JFH This is a hack to deal with the usage of this method in combineimage, which is not where it should be. + return None + def get_rawimage(self, raw_file, det): """ Read raw images and generate a few other bits and pieces From eba291fa85e447dca9be46a1735f27968afcfeef Mon Sep 17 00:00:00 2001 From: Kyle Westfall Date: Mon, 3 Jun 2024 17:11:07 -0700 Subject: [PATCH 06/14] test fix --- pypeit/images/combineimage.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pypeit/images/combineimage.py b/pypeit/images/combineimage.py index f66d327f04..49e5d3158e 100644 --- a/pypeit/images/combineimage.py +++ b/pypeit/images/combineimage.py @@ -149,7 +149,7 @@ def run(self, ignore_saturation=False, maxiters=5): if self.nimgs == 0: msgs.error('Object contains no files to process!') if self.nimgs > 1 and self.par['combine'] not in ['mean', 'median']: - msgs.error(f'Unknown image combination method, {self.par['combine']}. Must be ' + msgs.error(f'Unknown image combination method, {self.par["combine"]}. Must be ' '"mean" or "median".') file_list = [] # Loop on the files From 68aad1002bf07723788661681fc0b3e527b10f91 Mon Sep 17 00:00:00 2001 From: jhennawi Date: Sun, 9 Jun 2024 13:45:03 -0700 Subject: [PATCH 07/14] some updates --- pypeit/images/combineimage.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/pypeit/images/combineimage.py b/pypeit/images/combineimage.py index 49e5d3158e..587ff350f9 100644 --- a/pypeit/images/combineimage.py +++ b/pypeit/images/combineimage.py @@ -23,11 +23,10 @@ class CombineImage: Process and combine detector images. Args: - Parameters that dictate the processing of the images. - rawImages (:obj:`list`): - Either a single :class:`~pypeit.images.rawimage.RawImage` object or a list of one or more + rawImages (:obj:`list`, :class:`~pypeit.images.pypeitimage.PypeItImage): + Either a single :class:`~pypeit.images.pypeitimage.PypeItImage` object or a list of one or more of these objects to be combined into a an image. - spectrograph (:class:`~pypeit.spectrographs.spectrograph.Spectrograph`): + spectrograph (:class:`~pypeit.spectrographs.spectrograph.Spectrograph`): Spectrograph used to take the data. par (:class:`~pypeit.par.pypeitpar.ProcessImagesPar`): Parameters that dictate the processing of the images. From 1deaeb38ca9d09554eadd7c3c8059ef62b30b72e Mon Sep 17 00:00:00 2001 From: rcooke Date: Sat, 29 Jun 2024 13:26:45 +0100 Subject: [PATCH 08/14] Move lampstat check --- pypeit/images/buildimage.py | 27 +++++++++++++++++++++++++-- pypeit/images/combineimage.py | 28 ++-------------------------- pypeit/spectrographs/jwst_nirspec.py | 16 ---------------- 3 files changed, 27 insertions(+), 44 deletions(-) diff --git a/pypeit/images/buildimage.py b/pypeit/images/buildimage.py index dbd18d688b..e77da57a57 100644 --- a/pypeit/images/buildimage.py +++ b/pypeit/images/buildimage.py @@ -4,6 +4,8 @@ .. include:: ../include/links.rst """ +import os + from IPython import embed import numpy as np @@ -253,15 +255,36 @@ def buildimage_fromlist(spectrograph, det, frame_par, file_list, bias=None, bpm= mosaic = isinstance(det, tuple) and frame_par['frametype'] not in ['bias', 'dark'] rawImage_list = [] + lampstat = [None] * len(file_list) # Loop on the files - for ifile in file_list: + for ii, ifile in enumerate(file_list): # Load raw image rawImage = rawimage.RawImage(ifile, spectrograph, det) # Process rawImage_list.append(rawImage.process( frame_par['process'], scattlight=scattlight, bias=bias, bpm=bpm, dark=dark, flatimages=flatimages, slits=slits, mosaic=mosaic)) - + + # Save the lamp status + # TODO: Is there a way we can get this from fitstbl instead? + lampstat[ii] = spectrograph.get_lamps_status(rawImage_list[ii].rawheadlist) + + # Check that the lamps being combined are all the same: + if not lampstat[1:] == lampstat[:-1]: + msgs.warn("The following files contain different lamp status") + # Get the longest strings + maxlen = max([len("Filename")]+[len(os.path.split(x)[1]) for x in file_list]) + maxlmp = max([len("Lamp status")]+[len(x) for x in lampstat]) + strout = "{0:" + str(maxlen) + "} {1:s}" + # Print the messages + print(msgs.indent() + '-'*maxlen + " " + '-'*maxlmp) + print(msgs.indent() + strout.format("Filename", "Lamp status")) + print(msgs.indent() + '-'*maxlen + " " + '-'*maxlmp) + for ff, file in enumerate(file_list): + print(msgs.indent() + + strout.format(os.path.split(file)[1], " ".join(lampstat[ff].split("_")))) + print(msgs.indent() + '-'*maxlen + " " + '-'*maxlmp) + # Do it combineImage = combineimage.CombineImage(rawImage_list, spectrograph, frame_par['process']) pypeitImage = combineImage.run(maxiters=maxiters, ignore_saturation=ignore_saturation) diff --git a/pypeit/images/combineimage.py b/pypeit/images/combineimage.py index 587ff350f9..ffc7f8b990 100644 --- a/pypeit/images/combineimage.py +++ b/pypeit/images/combineimage.py @@ -4,8 +4,6 @@ .. include:: ../include/links.rst """ -import os - from IPython import embed import numpy as np @@ -18,6 +16,7 @@ from pypeit.images import pypeitimage from pypeit.images import imagebitmask + class CombineImage: """ Process and combine detector images. @@ -165,13 +164,8 @@ def run(self, ignore_saturation=False, maxiters=5): rn2img_stack = np.zeros(shape, dtype=float) basev_stack = np.zeros(shape, dtype=float) gpm_stack = np.zeros(shape, dtype=bool) - lampstat = [None]*self.nimgs exptime = np.zeros(self.nimgs, dtype=float) - # Save the lamp status - # TODO: As far as I can tell, this is the *only* place rawheadlist - # is used. Is there a way we can get this from fitstbl instead? - lampstat[kk] = self.spectrograph.get_lamps_status(rawImage.rawheadlist) # Save the exposure time to check if it's consistent for all images. exptime[kk] = rawImage.exptime # Processed image @@ -195,25 +189,7 @@ def run(self, ignore_saturation=False, maxiters=5): gpm_stack[kk] = rawImage.select_flag(invert=True) file_list.append(rawImage.filename) - # TODO JFH: This should not be here, but rather should be done in the Calibrations class. Putting in calibration specific - # lamp checking in an image combining class is not ideal. - # Check that the lamps being combined are all the same: - if not lampstat[1:] == lampstat[:-1]: - msgs.warn("The following files contain different lamp status") - # Get the longest strings - maxlen = max([len("Filename")]+[len(os.path.split(x)[1]) for x in self.files]) - maxlmp = max([len("Lamp status")]+[len(x) for x in lampstat]) - strout = "{0:" + str(maxlen) + "} {1:s}" - # Print the messages - print(msgs.indent() + '-'*maxlen + " " + '-'*maxlmp) - print(msgs.indent() + strout.format("Filename", "Lamp status")) - print(msgs.indent() + '-'*maxlen + " " + '-'*maxlmp) - for ff, file in enumerate(self.files): - print(msgs.indent() - + strout.format(os.path.split(file)[1], " ".join(lampstat[ff].split("_")))) - print(msgs.indent() + '-'*maxlen + " " + '-'*maxlmp) - - # Do a similar check for exptime + # Check that all exposure times are consistent if np.any(np.absolute(np.diff(exptime)) > 0): # TODO: This should likely throw an error instead! msgs.warn('Exposure time is not consistent for all images being combined! ' diff --git a/pypeit/spectrographs/jwst_nirspec.py b/pypeit/spectrographs/jwst_nirspec.py index 088670fa39..80a8abb2f3 100644 --- a/pypeit/spectrographs/jwst_nirspec.py +++ b/pypeit/spectrographs/jwst_nirspec.py @@ -254,22 +254,6 @@ def allowed_mosaics(self): ``PypeIt``. """ return [(1,2)] - - - - def get_lamps_status(self, headarr): - """ - Return a string containing the information on the lamp status. - - Args: - headarr (:obj:`list`): - A list of 1 or more `astropy.io.fits.Header`_ objects. - - Returns: - :obj:`str`: A string that uniquely represents the lamp status. - """ - # TODO: JFH This is a hack to deal with the usage of this method in combineimage, which is not where it should be. - return None def get_rawimage(self, raw_file, det): """ From 69700a3e9e481700de7a75c7f725b5c15a82600c Mon Sep 17 00:00:00 2001 From: rcooke Date: Mon, 1 Jul 2024 10:45:52 +0100 Subject: [PATCH 09/14] Move lampstat check to calibrations --- pypeit/calibrations.py | 79 +++++++++++++++++++++++++++++++++++ pypeit/images/buildimage.py | 23 ---------- pypeit/images/combineimage.py | 1 + 3 files changed, 80 insertions(+), 23 deletions(-) diff --git a/pypeit/calibrations.py b/pypeit/calibrations.py index e8875595df..998f9a9cff 100644 --- a/pypeit/calibrations.py +++ b/pypeit/calibrations.py @@ -4,6 +4,7 @@ .. include common links, assuming primary doc root is up one directory .. include:: ../include/links.rst """ +import os from pathlib import Path from datetime import datetime from copy import deepcopy @@ -209,6 +210,44 @@ def __init__(self, fitstbl, par, spectrograph, caldir, qadir=None, self.success = False self.failed_step = None + def check_calibrations(self, file_list, check_lamps=True): + """ + Check if the input calibration files are consistent with each other. + This step is usually needed when combining calibration frames of a given type. + + Note: The exposure times are currently checked in the combine step, so they are not checked here. + + Parameters + ---------- + file_list : list + List of calibration files to check + check_lamps : bool, optional + Check if the lamp status is the same for all the files. Default is True. + """ + lampstat = [None] * len(file_list) + # Loop on the files + for ii, ifile in enumerate(file_list): + # Save the lamp status + headarr = deepcopy(self.spectrograph.get_headarr(ifile)) + lampstat[ii] = self.spectrograph.get_lamps_status(headarr) + + # Check that the lamps being combined are all the same + if check_lamps: + if not lampstat[1:] == lampstat[:-1]: + msgs.warn("The following files contain different lamp status") + # Get the longest strings + maxlen = max([len("Filename")] + [len(os.path.split(x)[1]) for x in file_list]) + maxlmp = max([len("Lamp status")] + [len(x) for x in lampstat]) + strout = "{0:" + str(maxlen) + "} {1:s}" + # Print the messages + print(msgs.indent() + '-' * maxlen + " " + '-' * maxlmp) + print(msgs.indent() + strout.format("Filename", "Lamp status")) + print(msgs.indent() + '-' * maxlen + " " + '-' * maxlmp) + for ff, file in enumerate(file_list): + print(msgs.indent() + + strout.format(os.path.split(file)[1], " ".join(lampstat[ff].split("_")))) + print(msgs.indent() + '-' * maxlen + " " + '-' * maxlmp) + def find_calibrations(self, frametype, frameclass): """ Find calibration files and identifiers. @@ -334,6 +373,9 @@ def get_arc(self): # Reset the BPM self.get_bpm(frame=raw_files[0]) + # Perform a check on the files + self.check_calibrations(raw_files) + # Otherwise, create the processed file. msgs.info(f'Preparing a {frame["class"].calib_type} calibration frame.') self.msarc = buildimage.buildimage_fromlist(self.spectrograph, self.det, @@ -377,6 +419,9 @@ def get_tiltimg(self): # Reset the BPM self.get_bpm(frame=raw_files[0]) + # Perform a check on the files + self.check_calibrations(raw_files) + # Otherwise, create the processed file. msgs.info(f'Preparing a {frame["class"].calib_type} calibration frame.') self.mstilt = buildimage.buildimage_fromlist(self.spectrograph, self.det, @@ -426,6 +471,9 @@ def get_align(self): # Reset the BPM self.get_bpm(frame=raw_files[0]) + # Perform a check on the files + self.check_calibrations(raw_files) + # Otherwise, create the processed file. msgs.info(f'Preparing a {frame["class"].calib_type} calibration frame.') msalign = buildimage.buildimage_fromlist(self.spectrograph, self.det, @@ -473,6 +521,9 @@ def get_bias(self): self.msbias = frame['class'].from_file(cal_file, chk_version=self.chk_version) return self.msbias + # Perform a check on the files + self.check_calibrations(raw_files) + # Otherwise, create the processed file. msgs.info(f'Preparing a {frame["class"].calib_type} calibration frame.') self.msbias = buildimage.buildimage_fromlist(self.spectrograph, self.det, @@ -523,6 +574,9 @@ def get_dark(self): # there any reason why creation of the bpm should come after the dark, # or can we change the order? + # Perform a check on the files + self.check_calibrations(raw_files) + # Otherwise, create the processed file. self.msdark = buildimage.buildimage_fromlist(self.spectrograph, self.det, self.par['darkframe'], raw_files, @@ -597,6 +651,9 @@ def get_scattlight(self): # Reset the BPM self.get_bpm(frame=raw_scattlight_files[0]) + # Perform a check on the files + self.check_calibrations(raw_scattlight_files) + binning = self.fitstbl[scatt_idx[0]]['binning'] dispname = self.fitstbl[scatt_idx[0]]['dispname'] scattlightImage = buildimage.buildimage_fromlist(self.spectrograph, self.det, @@ -725,6 +782,10 @@ def get_flats(self): if len(raw_pixel_files) > 0: # Reset the BPM self.get_bpm(frame=raw_pixel_files[0]) + + # Perform a check on the files + self.check_calibrations(raw_pixel_files) + msgs.info('Creating pixel-flat calibration frame using files: ') for f in raw_pixel_files: msgs.prindent(f'{Path(f).name}') @@ -737,6 +798,10 @@ def get_flats(self): if len(raw_lampoff_files) > 0: # Reset the BPM self.get_bpm(frame=raw_lampoff_files[0]) + + # Perform a check on the files + self.check_calibrations(raw_lampoff_files) + msgs.info('Subtracting lamp off flats using files: ') for f in raw_lampoff_files: msgs.prindent(f'{Path(f).name}') @@ -763,6 +828,10 @@ def get_flats(self): if not pix_is_illum and len(raw_illum_files) > 0: # Reset the BPM self.get_bpm(frame=raw_illum_files[0]) + + # Perform a check on the files + self.check_calibrations(raw_illum_files) + msgs.info('Creating slit-illumination flat calibration frame using files: ') for f in raw_illum_files: msgs.prindent(f'{Path(f).name}') @@ -775,6 +844,10 @@ def get_flats(self): for f in raw_lampoff_files: msgs.prindent(f'{Path(f).name}') if lampoff_flat is None: + # Perform a check on the files + self.check_calibrations(raw_lampoff_files) + + # Build the image lampoff_flat = buildimage.buildimage_fromlist(self.spectrograph, self.det, self.par['lampoffflatsframe'], raw_lampoff_files, @@ -889,6 +962,9 @@ def get_slits(self): # Reset the BPM self.get_bpm(frame=raw_trace_files[0]) + # Perform a check on the files + self.check_calibrations(raw_trace_files) + traceImage = buildimage.buildimage_fromlist(self.spectrograph, self.det, self.par['traceframe'], raw_trace_files, bias=self.msbias, bpm=self.msbpm, @@ -903,6 +979,9 @@ def get_slits(self): # Reset the BPM self.get_bpm(frame=raw_trace_files[0]) + # Perform a check on the files + self.check_calibrations(raw_lampoff_files) + lampoff_flat = buildimage.buildimage_fromlist(self.spectrograph, self.det, self.par['lampoffflatsframe'], raw_lampoff_files, dark=self.msdark, diff --git a/pypeit/images/buildimage.py b/pypeit/images/buildimage.py index e77da57a57..74f894d4c3 100644 --- a/pypeit/images/buildimage.py +++ b/pypeit/images/buildimage.py @@ -4,8 +4,6 @@ .. include:: ../include/links.rst """ -import os - from IPython import embed import numpy as np @@ -255,7 +253,6 @@ def buildimage_fromlist(spectrograph, det, frame_par, file_list, bias=None, bpm= mosaic = isinstance(det, tuple) and frame_par['frametype'] not in ['bias', 'dark'] rawImage_list = [] - lampstat = [None] * len(file_list) # Loop on the files for ii, ifile in enumerate(file_list): # Load raw image @@ -265,26 +262,6 @@ def buildimage_fromlist(spectrograph, det, frame_par, file_list, bias=None, bpm= frame_par['process'], scattlight=scattlight, bias=bias, bpm=bpm, dark=dark, flatimages=flatimages, slits=slits, mosaic=mosaic)) - # Save the lamp status - # TODO: Is there a way we can get this from fitstbl instead? - lampstat[ii] = spectrograph.get_lamps_status(rawImage_list[ii].rawheadlist) - - # Check that the lamps being combined are all the same: - if not lampstat[1:] == lampstat[:-1]: - msgs.warn("The following files contain different lamp status") - # Get the longest strings - maxlen = max([len("Filename")]+[len(os.path.split(x)[1]) for x in file_list]) - maxlmp = max([len("Lamp status")]+[len(x) for x in lampstat]) - strout = "{0:" + str(maxlen) + "} {1:s}" - # Print the messages - print(msgs.indent() + '-'*maxlen + " " + '-'*maxlmp) - print(msgs.indent() + strout.format("Filename", "Lamp status")) - print(msgs.indent() + '-'*maxlen + " " + '-'*maxlmp) - for ff, file in enumerate(file_list): - print(msgs.indent() - + strout.format(os.path.split(file)[1], " ".join(lampstat[ff].split("_")))) - print(msgs.indent() + '-'*maxlen + " " + '-'*maxlmp) - # Do it combineImage = combineimage.CombineImage(rawImage_list, spectrograph, frame_par['process']) pypeitImage = combineImage.run(maxiters=maxiters, ignore_saturation=ignore_saturation) diff --git a/pypeit/images/combineimage.py b/pypeit/images/combineimage.py index ffc7f8b990..a1b45b3f6f 100644 --- a/pypeit/images/combineimage.py +++ b/pypeit/images/combineimage.py @@ -190,6 +190,7 @@ def run(self, ignore_saturation=False, maxiters=5): file_list.append(rawImage.filename) # Check that all exposure times are consistent + # TODO: JFH suggests that we move this to calibrations.check_calibrations if np.any(np.absolute(np.diff(exptime)) > 0): # TODO: This should likely throw an error instead! msgs.warn('Exposure time is not consistent for all images being combined! ' From 62ddd8a0a3d5f2f02be2f2d0cd31c0050bd22b22 Mon Sep 17 00:00:00 2001 From: jhennawi Date: Fri, 19 Jul 2024 21:27:18 +0900 Subject: [PATCH 10/14] Updated docs on check_calibraitons --- pypeit/calibrations.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pypeit/calibrations.py b/pypeit/calibrations.py index 998f9a9cff..9c89d4bf9f 100644 --- a/pypeit/calibrations.py +++ b/pypeit/calibrations.py @@ -214,6 +214,7 @@ def check_calibrations(self, file_list, check_lamps=True): """ Check if the input calibration files are consistent with each other. This step is usually needed when combining calibration frames of a given type. + This routine currently only prints out warning messages if the calibration files are not consistent. Note: The exposure times are currently checked in the combine step, so they are not checked here. @@ -224,6 +225,7 @@ def check_calibrations(self, file_list, check_lamps=True): check_lamps : bool, optional Check if the lamp status is the same for all the files. Default is True. """ + lampstat = [None] * len(file_list) # Loop on the files for ii, ifile in enumerate(file_list): From 0373f38a0a0fd26b55cf7bf3dbe252c68f2d3611 Mon Sep 17 00:00:00 2001 From: jhennawi Date: Fri, 19 Jul 2024 21:28:17 +0900 Subject: [PATCH 11/14] Tuned up buildimage to just loop over the file. --- pypeit/images/buildimage.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pypeit/images/buildimage.py b/pypeit/images/buildimage.py index 0d587f2f67..12050ea17e 100644 --- a/pypeit/images/buildimage.py +++ b/pypeit/images/buildimage.py @@ -254,7 +254,7 @@ def buildimage_fromlist(spectrograph, det, frame_par, file_list, bias=None, bpm= rawImage_list = [] # Loop on the files - for ii, ifile in enumerate(file_list): + for ifile in file_list: # Load raw image rawImage = rawimage.RawImage(ifile, spectrograph, det) # Process From 58391cdd14305c3f07b3d01af67db9aff1b47ec3 Mon Sep 17 00:00:00 2001 From: jhennawi Date: Fri, 19 Jul 2024 21:40:24 +0900 Subject: [PATCH 12/14] addressing PR comments. --- pypeit/images/buildimage.py | 2 +- pypeit/images/combineimage.py | 10 +++------- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/pypeit/images/buildimage.py b/pypeit/images/buildimage.py index 12050ea17e..14c4f3d1d9 100644 --- a/pypeit/images/buildimage.py +++ b/pypeit/images/buildimage.py @@ -263,7 +263,7 @@ def buildimage_fromlist(spectrograph, det, frame_par, file_list, bias=None, bpm= bpm=bpm, dark=dark, flatimages=flatimages, slits=slits, mosaic=mosaic)) # Do it - combineImage = combineimage.CombineImage(rawImage_list, spectrograph, frame_par['process']) + combineImage = combineimage.CombineImage(rawImage_list, frame_par['process']) pypeitImage = combineImage.run(maxiters=maxiters, ignore_saturation=ignore_saturation) # Return class type, if returning any of the frame_image_classes cls = frame_image_classes[frame_par['frametype']] \ diff --git a/pypeit/images/combineimage.py b/pypeit/images/combineimage.py index a1b45b3f6f..9bf3ce33c4 100644 --- a/pypeit/images/combineimage.py +++ b/pypeit/images/combineimage.py @@ -25,14 +25,10 @@ class CombineImage: rawImages (:obj:`list`, :class:`~pypeit.images.pypeitimage.PypeItImage): Either a single :class:`~pypeit.images.pypeitimage.PypeItImage` object or a list of one or more of these objects to be combined into a an image. - spectrograph (:class:`~pypeit.spectrographs.spectrograph.Spectrograph`): - Spectrograph used to take the data. par (:class:`~pypeit.par.pypeitpar.ProcessImagesPar`): Parameters that dictate the processing of the images. Attributes: - spectrograph (:class:`~pypeit.spectrographs.spectrograph.Spectrograph`): - Spectrograph used to take the data. det (:obj:`int`, :obj:`tuple`): The 1-indexed detector number(s) to process. par (:class:`~pypeit.par.pypeitpar.ProcessImagesPar`): @@ -42,11 +38,10 @@ class CombineImage: to be combined. """ - def __init__(self, rawImages, spectrograph, par): + def __init__(self, rawImages, par): if not isinstance(par, pypeitpar.ProcessImagesPar): msgs.error('Provided ParSet for must be type ProcessImagesPar.') self.rawImages = list(rawImages) if hasattr(rawImages, '__len__') else [rawImages] - self.spectrograph = spectrograph self.par = par # This musts be named this way as it is frequently a child # NOTE: nimgs is a property method. Defining rawImages above must come @@ -244,6 +239,7 @@ def run(self, ignore_saturation=False, maxiters=5): noise_floor=self.par['noise_floor']) # Build the combined image + embed() comb = pypeitimage.PypeItImage(image=comb_img, ivar=utils.inverse(comb_var), nimg=nframes, amp_img=rawImage.amp_img, det_img=rawImage.det_img, rn2img=comb_rn2, base_var=comb_basev, img_scale=comb_scl, @@ -252,7 +248,7 @@ def run(self, ignore_saturation=False, maxiters=5): # NOTE: The detector is needed here so # that we can get the dark current later. detector=rawImage.detector, - PYP_SPEC=self.spectrograph.name, + PYP_SPEC=rawImage.PYP_SPEC, units='e-' if self.par['apply_gain'] else 'ADU', exptime=comb_texp, noise_floor=self.par['noise_floor'], shot_noise=self.par['shot_noise']) From 13f0dc4bb88bd1d5d93e0701cbd95c519fd58d5b Mon Sep 17 00:00:00 2001 From: jhennawi Date: Fri, 19 Jul 2024 21:55:40 +0900 Subject: [PATCH 13/14] addressing PR comments. --- pypeit/images/pypeitimage.py | 4 ++++ pypeit/images/rawimage.py | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/pypeit/images/pypeitimage.py b/pypeit/images/pypeitimage.py index 79ea14187d..dc8e07b591 100644 --- a/pypeit/images/pypeitimage.py +++ b/pypeit/images/pypeitimage.py @@ -91,6 +91,9 @@ class PypeItImage(datamodel.DataContainer): :class:`~pypeit.images.rawimage.RawImage.process`. """ + # TODO These docs are confusing. The __init__ method needs to be documented just as it is for + # every other class that we have written in PypeIt, i.e. the arguments all need to be documented. They are not + # documented here and instead we have the odd Args documentation above. version = '1.3.0' """Datamodel version number""" @@ -161,6 +164,7 @@ def from_pypeitimage(cls, pypeitImage): # Done return self + # TODO This init method needs proper docs, which includes every optional argument. See my comment above. def __init__(self, image, ivar=None, nimg=None, amp_img=None, det_img=None, rn2img=None, base_var=None, img_scale=None, fullmask=None, detector=None, spat_flexure=None, filename=None, PYP_SPEC=None, units=None, exptime=None, noise_floor=None, diff --git a/pypeit/images/rawimage.py b/pypeit/images/rawimage.py index 1afaca5e40..831a9a7c94 100644 --- a/pypeit/images/rawimage.py +++ b/pypeit/images/rawimage.py @@ -676,9 +676,9 @@ def process(self, par, bpm=None, scattlight=None, flatimages=None, bias=None, sl exptime=self.exptime, noise_floor=self.par['noise_floor'], shot_noise=self.par['shot_noise'], - bpm=_bpm.astype(bool)) + bpm=_bpm.astype(bool), + filename=self.filename) - pypeitImage.filename = self.filename pypeitImage.rawheadlist = self.headarr pypeitImage.process_steps = [key for key in self.steps.keys() if self.steps[key]] From 461260febdd23fbf37e8c38a6f1113927ad0af6d Mon Sep 17 00:00:00 2001 From: jhennawi Date: Fri, 19 Jul 2024 22:08:27 +0900 Subject: [PATCH 14/14] adressing pr comments --- pypeit/images/combineimage.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pypeit/images/combineimage.py b/pypeit/images/combineimage.py index 9bf3ce33c4..635cb4905d 100644 --- a/pypeit/images/combineimage.py +++ b/pypeit/images/combineimage.py @@ -239,7 +239,6 @@ def run(self, ignore_saturation=False, maxiters=5): noise_floor=self.par['noise_floor']) # Build the combined image - embed() comb = pypeitimage.PypeItImage(image=comb_img, ivar=utils.inverse(comb_var), nimg=nframes, amp_img=rawImage.amp_img, det_img=rawImage.det_img, rn2img=comb_rn2, base_var=comb_basev, img_scale=comb_scl,