Adding support for the (decommissioned) spectrograph: AAT/UHRF #1834

rcooke-ast · 2024-07-31T15:21:49Z

There is an accompanying dev-suite PR. I will also add some documentation about this spectrograph. Tests currently pass.

# Conflicts: # doc/releases/1.15.1dev.rst

codecov-commenter · 2024-07-31T15:32:02Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 48.29932% with 76 lines in your changes missing coverage. Please review.

Project coverage is 38.03%. Comparing base (745f36c) to head (905d775).

Files with missing lines	Patch %	Lines
pypeit/spectrographs/aat_uhrf.py	30.43%	64 Missing ⚠️
pypeit/spectrographs/spectrograph.py	63.63%	4 Missing ⚠️
pypeit/io.py	85.71%	2 Missing ⚠️
pypeit/scripts/obslog.py	0.00%	2 Missing ⚠️
pypeit/pypeitsetup.py	87.50%	1 Missing ⚠️
pypeit/scripts/chk_for_calibs.py	0.00%	1 Missing ⚠️
pypeit/scripts/setup_gui.py	0.00%	1 Missing ⚠️
pypeit/spectrographs/keck_kcwi.py	0.00%	1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1834      +/-   ##
===========================================
+ Coverage    38.01%   38.03%   +0.01%     
===========================================
  Files          210      211       +1     
  Lines        48850    48975     +125     
===========================================
+ Hits         18569    18626      +57     
- Misses       30281    30349      +68

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

profxj

All good on my end. I hear @kbwestfall will have one fix
to suggest/add.

profxj · 2024-08-12T21:38:37Z

pypeit/pypeitsetup.py

@@ -186,7 +186,20 @@ def from_file_root(cls, root, spectrograph, extension='.fits'):
        Returns:
            :class:`PypeitSetup`: The instance of the class.
        """
-        return cls.from_rawfiles(io.files_from_extension(root, extension=extension), spectrograph)
+        spec = load_spectrograph(spectrograph)


I hear that @kbwestfall has some issues/fixes for this

# Conflicts: # doc/releases/1.16.1dev.rst

Refactor file discovery

kbwestfall

There are a couple of somewhat buried changes that I was hoping you could comment on. I merged in release and develop. Can we re-run tests?

kbwestfall · 2024-08-26T16:47:26Z

pypeit/core/arc.py

+            maxi = min(nmask[0], ww.size)
+            outbpm[pks[i]:ww[maxi]+2] = True
+    # Return the output bpm
+    return outbpm


It would be great to have a test for this, but I'm fine with you punting on that for this PR.

Good point - I've added one in.

kbwestfall · 2024-08-26T16:51:10Z

pypeit/core/findobj_skymask.py

    flux_smash_mean, flux_smash_med, flux_smash_std = astropy.stats.sigma_clipped_stats(
-        flux_smash, mask=np.logical_not(gpm_smash), sigma_lower=3.0, sigma_upper=3.0
-    )
+        flux_smash, mask=smash_mask, sigma_lower=3.0, sigma_upper=3.0)


I haven't dug into the context of this change, but it feels important. Can you give us more information about the problem that you've addressed with this code? And, do you have any assessments of how this affects all the other instruments?

To be honest, I developed this many months ago, and I can't remember the motivation, or why it wasn't working in the first place. The code is basically finding peaks and masking around them, provided that pixel values continuously decrease away from the peak pixel. There needs to be a continuous decrease of 5+ pixels in order to activate this feature. I suspect this is only important when object profiles are excessively broad (which is what I was dealing with at the time).

Given that I can't recall what the motivation was, I am happy to revert this.

kbwestfall · 2024-08-26T16:55:04Z

pypeit/spectrographs/keck_kcwi.py

@@ -860,7 +860,7 @@ def get_detector_par(self, det, hdu=None):
            # Some properties of the image
            binning = self.compound_meta(self.get_headarr(hdu), "binning")
            numamps = hdu[0].header['NVIDINP']
-            specflip = True if hdu[0].header['AMPID1'] == 2 else False
+            specflip = False if hdu[0].header['AMPMODE'] == 'ALL' else True


I would comment on this in the change log. Is this a general change for all KCWI taken, or does it reflect a change in header that needs an associated date if..else check?

I was finding a problem with one of the datasets I have. This is effectively the same thing, but it depends which 2 amplifiers are used. This is more conservative. I have added this to the change log.

rcooke-ast

Comments addressed @kbwestfall - thanks! I'll run tests

rcooke-ast · 2024-08-26T17:26:23Z

pypeit/core/arc.py

+            maxi = min(nmask[0], ww.size)
+            outbpm[pks[i]:ww[maxi]+2] = True
+    # Return the output bpm
+    return outbpm


Good point - I've added one in.

rcooke-ast · 2024-08-26T17:29:20Z

pypeit/spectrographs/keck_kcwi.py

@@ -860,7 +860,7 @@ def get_detector_par(self, det, hdu=None):
            # Some properties of the image
            binning = self.compound_meta(self.get_headarr(hdu), "binning")
            numamps = hdu[0].header['NVIDINP']
-            specflip = True if hdu[0].header['AMPID1'] == 2 else False
+            specflip = False if hdu[0].header['AMPMODE'] == 'ALL' else True


I was finding a problem with one of the datasets I have. This is effectively the same thing, but it depends which 2 amplifiers are used. This is more conservative. I have added this to the change log.

rcooke-ast · 2024-08-26T19:16:59Z

pypeit/core/findobj_skymask.py

    flux_smash_mean, flux_smash_med, flux_smash_std = astropy.stats.sigma_clipped_stats(
-        flux_smash, mask=np.logical_not(gpm_smash), sigma_lower=3.0, sigma_upper=3.0
-    )
+        flux_smash, mask=smash_mask, sigma_lower=3.0, sigma_upper=3.0)


To be honest, I developed this many months ago, and I can't remember the motivation, or why it wasn't working in the first place. The code is basically finding peaks and masking around them, provided that pixel values continuously decrease away from the peak pixel. There needs to be a continuous decrease of 5+ pixels in order to activate this feature. I suspect this is only important when object profiles are excessively broad (which is what I was dealing with at the time).

Given that I can't recall what the motivation was, I am happy to revert this.

rcooke-ast · 2024-08-27T18:35:56Z

Tests pass

# Conflicts: # doc/releases/1.16.1dev.rst

rcooke-ast and others added 21 commits January 25, 2024 17:26

start of AAT

e0542b8

start of UHRF

3ddc81b

allow search for multiple extensions

a9d1f99

unique filenames only

62c9feb

meta adjusted

6ec9a02

wavelength calibration

5ea5866

better wavelength solution

23b8d50

minor doc updates

367c6d4

parameter updates

13a8cca

CR update

be3aafc

improved object finding for short slits

3f2698a

rm debug

c48753c

logical not

d0b2019

improved tilts

13fc394

Merge branch 'develop' into aat_uhrf

a1475b4

AAT/UHRF support

e37a6c9

Merge branch 'develop' into aat_uhrf

d421b94

# Conflicts: # doc/releases/1.15.1dev.rst

update changes

ed7001c

Merge branch 'develop' into aat_uhrf

fe7857b

add AAT/UHRF

c9029e6

add AAT/UHRF wavelength solution (CN)

76af9f7

rcooke-ast mentioned this pull request Jul 31, 2024

Adding a setup for AAT/UHRF pypeit/PypeIt-development-suite#327

Merged

rcooke-ast and others added 4 commits July 31, 2024 20:15

doc updates

584d83a

interrupt

f12276c

before testing

15db2c3

flip check

76c5d30

profxj approved these changes Aug 12, 2024

View reviewed changes

kbwestfall added 2 commits August 12, 2024 15:26

switch instance test

7f59d94

fix file extensions

23d471a

rcooke-ast and others added 16 commits August 13, 2024 14:56

Merge branch 'develop' into aat_uhrf

a5068b6

test + doc fixes

75aa277

Merge branch 'develop' into filesearch

29e02b7

Merge branch 'aat_uhrf' into filesearch

8b93ddc

doc update

348743f

test fix

6ceec23

Merge remote-tracking branch 'origin/filesearch' into orient_fix

f818bed

update extensions

4c322b8

allow fits.gz

dd282af

Merge branch 'develop' into aat_uhrf

66ac429

Merge branch 'develop' into filesearch

20bb2f0

# Conflicts: # doc/releases/1.16.1dev.rst

Merge branch 'aat_uhrf' into filesearch

9ea1bef

update users Aug 2024

49d2233

Merge pull request #1839 from pypeit/filesearch

72bf70f

Refactor file discovery

Merge branch 'release' into aat_uhrf

9ccda6b

doc update

67e4684

kbwestfall requested changes Aug 26, 2024

View reviewed changes

kbwestfall and others added 4 commits August 26, 2024 10:22

Merge branch 'develop' into aat_uhrf

1cd3ee0

updated change log for KCWI

7bb82ea

add test for mask around peaks

7e672af

revert to old masking

884af8f

rcooke-ast commented Aug 26, 2024

View reviewed changes

rm old arc line

a9e5cd7

rcooke-ast added 2 commits August 27, 2024 20:51

Merge branch 'develop' into aat_uhrf

3aec173

# Conflicts: # doc/releases/1.16.1dev.rst

Merge branch 'develop' into aat_uhrf

905d775

# Conflicts: # doc/releases/1.16.1dev.rst

kbwestfall approved these changes Aug 30, 2024

View reviewed changes

kbwestfall merged commit bd77c38 into develop Aug 30, 2024
23 checks passed

kbwestfall deleted the aat_uhrf branch August 30, 2024 17:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding support for the (decommissioned) spectrograph: AAT/UHRF #1834

Adding support for the (decommissioned) spectrograph: AAT/UHRF #1834

rcooke-ast commented Jul 31, 2024

codecov-commenter commented Jul 31, 2024 •

edited

Loading

profxj left a comment

profxj Aug 12, 2024

kbwestfall left a comment

kbwestfall Aug 26, 2024

rcooke-ast Aug 26, 2024

kbwestfall Aug 26, 2024

rcooke-ast Aug 26, 2024

kbwestfall Aug 26, 2024

rcooke-ast Aug 26, 2024

rcooke-ast left a comment

rcooke-ast Aug 26, 2024

rcooke-ast Aug 26, 2024

rcooke-ast Aug 26, 2024

rcooke-ast commented Aug 27, 2024

Adding support for the (decommissioned) spectrograph: AAT/UHRF #1834

Adding support for the (decommissioned) spectrograph: AAT/UHRF #1834

Conversation

rcooke-ast commented Jul 31, 2024

codecov-commenter commented Jul 31, 2024 • edited Loading

Codecov Report

profxj left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kbwestfall left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rcooke-ast left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rcooke-ast commented Aug 27, 2024

codecov-commenter commented Jul 31, 2024 •

edited

Loading