Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

GMOS SOUTH CCD upgrade and some other fixes #1798

Merged
merged 5 commits into from
Apr 18, 2024
Merged

GMOS SOUTH CCD upgrade and some other fixes #1798

Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
34137da
updateCCD+other_fixes
debora-pe Apr 11, 2024
7bc96d0
Merge branch 'develop' into gmoss_ccdupdate
debora-pe Apr 13, 2024
52c0425
fix manual extraction for mosaic
debora-pe Apr 16, 2024
d0420ff
comments
debora-pe Apr 18, 2024
f131b07
fix detid
debora-pe Apr 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 55 additions & 14 deletions pypeit/spectrographs/gemini_gmos.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from astropy.table import Table
from astropy.coordinates import SkyCoord
from astropy import units
from astropy import time
from astropy.wcs import wcs
from astropy.io import fits

Expand Down Expand Up @@ -89,6 +90,14 @@ class GeminiGMOSMosaicLookUp:
(4096, 0): {'shift': (2109., -0.73),
'rotation': 0.}
},
'BI11-41-4k-2,BI13-19-4k-3,BI12-34-4k-1': {'default_shape': (2048, 4224),
(0, 0): {'shift': (-2112.5, -1.1),
'rotation': 0.},
(2048, 0): {},
(4096, 0): {
'shift': (2110.8, 1.05),
'rotation': 0.}
},
}


Expand Down Expand Up @@ -116,7 +125,7 @@ def init_meta(self):
self.meta['decker'] = dict(ext=0, card='MASKNAME')
self.meta['binning'] = dict(card=None, compound=True) # Uses CCDSUM

self.meta['mjd'] = dict(ext=0, card='OBSEPOCH')
self.meta['mjd'] = dict(card=None, compound=True)
self.meta['exptime'] = dict(ext=0, card='EXPTIME')
self.meta['airmass'] = dict(ext=0, card='AIRMASS')
# Extras for config and frametyping
Expand Down Expand Up @@ -145,14 +154,20 @@ def compound_meta(self, headarr, meta_key):
# binning in the raw frames
ccdsum = headarr[1].get('CCDSUM')
if ccdsum is not None:
binspatial, binspec = parse.parse_binning(ccdsum)
binspec, binspatial = parse.parse_binning(ccdsum)
kbwestfall marked this conversation as resolved.
Show resolved Hide resolved
binning = parse.binning2string(binspec, binspatial)
else:
# binning in the spec2d file
binning = headarr[0].get('BINNING')
if binning is None:
msgs.error('Binning not found')
return binning
elif meta_key == 'mjd':
obsepoch = headarr[0].get('OBSEPOCH')
if obsepoch is not None:
return time.Time(obsepoch, format='jyear').mjd
else:
return None
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should update the docstring as well, because you are return None instead of a Metadata value read from the header(s)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. I also wonder if it might be worthwhile throwing an error, or, throw a warning and then assume that the observations have ben taken since "2023-12-14".

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We likely do not want it to throw an error because this would cause pypeit_setup to fault, which we want to avoid. Throwing a warning makes sense, and we can add that the return can be None.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, and I agree, but returning None could cause an error on line 846 (and several other places in this file where a possible None value is not checked)... I think?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yes, you're right. @debora-pe , you'll need to catch the case @rcooke-ast pointed out. There are other places in the code that can catch a None mjd (e.g., starting at line 255 in metadata.py), but maybe not everywhere.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided to use a dummy mjd (using today as date) and throwing a warning. What do you think?


def config_independent_frames(self):
"""
Expand Down Expand Up @@ -511,7 +526,9 @@ def get_mosaic_par(self, mosaic, hdu=None, msc_order=0):
for i, d in enumerate([det-1 for det in mosaic]):
msc_sft[i] = shift[d]
msc_rot[i] = rotation[d]
msc_tfm[i] = build_image_mosaic_transform(shape, msc_sft[i], msc_rot[i], binning)
# binning is here in the PypeIt convention of (binspec, binspat), but the mosaic tranformations
# occur in the raw data frame, which has spatial in y and spectral in x
msc_tfm[i] = build_image_mosaic_transform(shape, msc_sft[i], msc_rot[i], tuple(reversed(binning)))
kbwestfall marked this conversation as resolved.
Show resolved Hide resolved

return Mosaic(mosaic_id, detectors, shape, np.array(msc_sft), np.array(msc_rot),
np.array(msc_tfm), msc_order)
Expand Down Expand Up @@ -738,7 +755,7 @@ class GeminiGMOSSHamSpectrograph(GeminiGMOSSpectrograph):
telescope = telescopes.GeminiSTelescopePar()
supported = True
comment = 'Hamamatsu detector (R400, B600, R831); see :doc:`gemini_gmos`'
detid = 'BI5-36-4k-2,BI11-33-4k-1,BI12-34-4k-1'
detid = 'BI5-36-4k-2,BI11-33-4k-1,BI12-34-4k-1' # this is changed in get_detector_par if t_obs >= t_upgrade
kbwestfall marked this conversation as resolved.
Show resolved Hide resolved

def hdu_read_order(self):
"""
Expand Down Expand Up @@ -784,8 +801,8 @@ def get_detector_par(self, det, hdu=None):
nonlinear = 0.95,
mincounts = -1e10,
numamplifiers = 4,
gain = np.atleast_1d([1.83]*4),
ronoise = np.atleast_1d([3.98]*4),
gain = np.atleast_1d([1.86]*4), # Slow Readout, Low Gain
ronoise = np.atleast_1d([4.19]*4), # Slow Readout, Low Gain
)
# Detector 2
detector_dict2 = dict(
Expand All @@ -801,8 +818,8 @@ def get_detector_par(self, det, hdu=None):
nonlinear = 0.95,
mincounts = -1e10,
numamplifiers = 4,
gain = np.atleast_1d([1.83]*4),
ronoise = np.atleast_1d([3.98]*4),
gain = np.atleast_1d([1.89]*4), # Slow Readout, Low Gain
ronoise = np.atleast_1d([4.13]*4), # Slow Readout, Low Gain
)
# Detector 3
detector_dict3 = dict(
Expand All @@ -818,9 +835,33 @@ def get_detector_par(self, det, hdu=None):
nonlinear = 0.95,
mincounts = -1e10,
numamplifiers = 4,
gain = np.atleast_1d([1.83]*4),
ronoise = np.atleast_1d([3.98]*4),
gain = np.atleast_1d([1.74]*4), # Slow Readout, Low Gain
ronoise = np.atleast_1d([3.75]*4), # Slow Readout, Low Gain
)

# account for the CCD upgrade happened on 2023-12-14
if hdu is not None:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this perhaps be defined before the detector dictionaries? (i.e. define parameters gain1, gain2, etc...). I think it's fine to update, but might be cleaner if we set variables near the top of this function and use those variables in the dictionary.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do actually prefer to leave it like this. Other spectrographs have the same flow. Also, the code below is only run if hdu is provided, which would make the code full of if...else if I define the parameters before the dictionary.

# date upgrade
t_upgrade = time.Time("2023-12-14", format='isot')
obs_date = time.Time(self.get_meta_value(self.get_headarr(hdu), 'mjd'), format='mjd')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be None sometimes?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comment above


if obs_date >= t_upgrade:
# These values are taken from
# https://github.com/GeminiDRSoftware/DRAGONS/blob/v3.2.0/gemini_instruments/gmos/lookup.py
# For each detector, we use the values averaged over the 4 amps for Slow Readout+Low Gain
detector_dict1['gain'] = np.atleast_1d([2.00]*4)
detector_dict2['gain'] = np.atleast_1d([1.78]*4)
detector_dict3['gain'] = np.atleast_1d([1.69]*4)
detector_dict1['ronoise'] = np.atleast_1d([4.19]*4)
detector_dict2['ronoise'] = np.atleast_1d([3.82]*4)
detector_dict3['ronoise'] = np.atleast_1d([3.50]*4)
# TODO: we may need to update the saturation values

# Update the detector ID
self.detid = 'BI11-41-4k-2,BI13-19-4k-3,BI12-34-4k-1'
msgs.info(f'Using the detector parameters for GMOS-S Hamamatsu after the upgrade on '
f'{t_upgrade.iso.split(" ")[0]}')

detectors = [detector_dict1, detector_dict2, detector_dict3]
# Return
return detector_container.DetectorContainer(**detectors[det-1])
Expand Down Expand Up @@ -884,7 +925,7 @@ def bpm(self, filename, det, shape=None, msbias=None):
# TODO: We're opening the file too many times...
hdrs = self.get_headarr(filename)
binning = self.get_meta_value(hdrs, 'binning')
obs_epoch = self.get_meta_value(hdrs, 'mjd')
obs_epoch = time.Time(self.get_meta_value(hdrs, 'mjd'), format='mjd').jyear
bin_spec, bin_spat= parse.parse_binning(binning)

# Add the detector-specific, hard-coded bad columns
Expand All @@ -906,7 +947,7 @@ def bpm(self, filename, det, shape=None, msbias=None):
_bpm_img[i,badr,:] = 1
# Bad amp as of January 28, 2022
# https://gemini.edu/sciops/instruments/gmos/GMOS-S_badamp5_ops_3.pdf
if obs_epoch > 2022.07:
if (obs_epoch > 2022.07) & (obs_epoch < time.Time("2023-12-14", format='isot').jyear):
kbwestfall marked this conversation as resolved.
Show resolved Hide resolved
badr = (768*2)//bin_spec
_bpm_img[i,badr:,:] = 1
if 3 in _det:
Expand Down Expand Up @@ -945,10 +986,10 @@ def config_specific_par(self, scifile, inp_par=None):
par['calibrations']['wavelengths']['method'] = 'reidentify'

# The bad amp needs a larger follow_span for slit edge tracing
obs_epoch = self.get_meta_value(scifile, 'mjd')
obs_epoch = time.Time(self.get_meta_value(scifile, 'mjd'), format='mjd').jyear
binning = self.get_meta_value(scifile, 'binning')
bin_spec, bin_spat= parse.parse_binning(binning)
if obs_epoch > 2022.07:
if (obs_epoch > 2022.07) & (obs_epoch < time.Time("2023-12-14", format='isot').jyear):
kbwestfall marked this conversation as resolved.
Show resolved Hide resolved
par['calibrations']['slitedges']['follow_span'] = 290*bin_spec
#
return par
Expand Down
Loading