CTE fix: fit offset per CCD sector as part of sky subtraction #1571

julienguy · 2022-01-05T23:01:50Z

This PR adds an option to fit spectral offsets for fiber/wavelength corresponding to predefined CCD sectors as part of the sky subtraction. The idea is to use this tool to correct for the charge transfer issues that lead to a net offset in the spectral flux of some fibers and wavelength.

Modify function compute_uniform_sky with option fit_offsets
- Code looks for keyword OFFCOLSX with X being an amplifier label in the calib yaml file. For instance, in /global/cfs/cdirs/desi/users/jguy/teststand/desi_spectro_calib/spec/sm10/sm10-r.yaml,

  ...
  # fibers have a flux offset because of a CTE issue in amplifier D
  # plot-amp-cte.py -i ~/desi/spectro/nightwatch/nersc/20211222/00115204/preproc-r1-00115204.fits  --xminmax 0 4100
  # shows that the charge trap starts at x=3714
  OFFCOLSD: '2057:3715'

Use range of columns to define a list of sectors with offsets
Use sky residuals in sky fibers to determine the offsets (2 parameters per CCD sector: value inside and outside of the sector)
Add option --fit-offsets to desi_compute_sky
Use this option by default in desi_proc

Example:

desi_proc -n 20211222 -e 115156 --nostdstarfit --nofluxcalib --cam r1 

...

INFO:sky.py:704:compute_uniform_sky: Adding CCD sector in amp D with offset: [2064, 4128, 2057, 3715]
INFO:sky.py:752:compute_uniform_sky: best fit offsets=[-3.0079104371238796, 1.068591454959013]

plot_frame -i ~/redux/jguy/exposures/20211222/00115156/sframe-r1-00115156.fits --sky-fibers --rebin 100

to be compared with the same plot using the daily prod

…ky subtraction

coveralls · 2022-01-05T23:12:20Z

Coverage decreased (-0.03%) to 25.469% when pulling f7ac942 on offset-per-sector-skysub into 5859c62 on master.

julienguy · 2022-01-05T23:20:33Z

In order to test the code for r1, use at NERSC: export DESI_SPECTRO_CALIB=/global/cfs/cdirs/desi/users/jguy/teststand/desi_spectro_calib .
We could also test the earlier occurence of the CTE issue on this camera but this would require to add to the yaml file a specific config for this period of Sept-Oct 2021.

schlafly

I wanted to understand better how the pipeline flow worked and so I thought I'd stare at this PR. Please ignore because I'm probably way off base!

schlafly · 2022-01-06T00:21:57Z

py/desispec/sky.py

+        psf_filename = findfile('psf',frame.meta["NIGHT"],frame.meta["EXPID"],frame.meta["CAMERA"])
+        if not os.path.isfile(psf_filename) :
+            log.error("No PSF file "+psf_filename)
+            raise IOError("No PSF file "+psf_filename)
+        log.info("Using PSF {}".format(psf_filename))
+        tset = read_xytraceset(psf_filename)
+
+        tmp_fibers = np.arange(frame.nspec)
+        tmp_x = np.zeros(frame.flux.shape,dtype=float)
+        tmp_y = np.zeros(frame.flux.shape,dtype=float)
+        for fiber in tmp_fibers :
+            tmp_x[fiber] = tset.x_vs_wave(fiber=fiber,wavelength=frame.wave)
+            tmp_y[fiber] = tset.y_vs_wave(fiber=fiber,wavelength=frame.wave)


Apologies as I read desispec code for the first time. Does this duplicate lines 714+ below and not actually get used?

yes. thank you.

py/desispec/sky.py

sbailey · 2022-01-06T04:48:38Z

I'm running a test prod on 20211222 and 20220102 using this branch in /global/cfs/cdirs/desi/users/sjbailey/spectro/redux/skycte. I post-facto realized that it was overkill to submit it for all cameras instead of just r1 which has the OFFCOLSX calib keyword, but desi_run_night doesn't have a --cameras option to make that easy, so consider this to also be a generic processing robustness test...

Notes to self (updated for second launch):

godesi test-master && module swap desispec/sjb3
export DESI_SPECTRO_CALIB=/global/cfs/cdirs/desi/users/jguy/teststand/desi_spectro_calib

myprod skycte
cdprod
copyprod $rx/daily . --night 20211222
copyprod $rx/daily . --night 20220102

#- flag non-sp1 cameras as bad to only run sp1
desi_edit_exposure_table -n 20211222 -e 115078:115275 -c BADCAMWORD -v a023456789
desi_edit_exposure_table -n 20220102 -e 116344:116452 -c BADCAMWORD -v a023456789

#- reuse extractions, redo other steps
for prefix in cframe calibstars exposure-qa fluxcalib sframe sky stdstars; do
    rm exposures/*/*/${prefix}*.*
done

desi_run_night --proc-obstypes science --z-submit-types pernight --system-name cori-knl -q regular -n 20211222
desi_run_night --proc-obstypes science --z-submit-types pernight --system-name cori-knl -q regular -n 20220102

julienguy · 2022-01-06T15:22:03Z

We will have to rerun this test prod after algorithmic changes. But it's interesting to see how the current implementation performs.

julienguy · 2022-01-06T17:53:38Z

I have committed a modified approach to the fit.

iterative fit (one sector at a time), fitting offset in and out of sector in same Y range
save background model bkg = sum_sectors (offset_in-offset_out)*mask_in
remove temporarily bkg to frame.flux
rerun sky subtraction
return the new fitted sky + bkg

This change improves the result on sframe-r1-00115156.fits
(primarily because of the error I had to consider the whole Y range for the 'outside' of a sector).

sbailey · 2022-01-06T18:52:32Z

FTR I canceled the test prod and resubmitted with the lastest update, re-using the extractions from daily and re-processing only b1,r1,z1 .

sbailey · 2022-01-11T03:57:36Z

The original "skycte" has been replaced with a "skycte2" production in:

/global/cfs/cdirs/desi/users/sjbailey/spectro/redux/skycte2

covering data on 20211222 and 20220102. sp1/r1 is the CCD with the CTE problem that hopefully is corrected with this test production. Please help evaluate. Night QA plots are at

https://data.desi.lbl.gov/desi/users/sjbailey/spectro/redux/skycte2/nightqa/20211222/nightqa-20211222.html
https://data.desi.lbl.gov/desi/users/sjbailey/spectro/redux/skycte2/nightqa/20220102/nightqa-20220102.html

Note that this branch was before the fiberflat humidity corrections, so there will be sframesky residuals.

schlafly · 2022-01-11T17:52:54Z

The petal n(z) PDF failed to generate for both of these; I was hoping to see that P1 improved to its post-CTE-fix level after applying this PR. Any chance that's an easy fix?

schlafly · 2022-01-11T17:58:45Z

And the first thing I see comparing the sframes from the nightly and prod sframe images is that the prod sframe image is visually much noisier. It fixes the offsets on P1, so the part that should work is working, but despite the sframe images claiming to have the same scale, there's a lot of extra noise. That's not expected?

ashleyjross · 2022-01-11T18:03:26Z

I will turn that notebook I made to compare the ampbkg results to the original into a general script that allows comparison between original and new reductions. Cheers, Ashley From: Eddie Schlafly ***@***.***> Date: Tuesday, January 11, 2022 at 12:53 PM To: desihub/desispec ***@***.***> Cc: Subscribed ***@***.***> Subject: Re: [desihub/desispec] Fit offset per CCD sector as part of sky subtraction (PR #1571) The petal n(z) PDF failed to generate for both of these; I was hoping to see that P1 improved to its post-CTE-fix level after applying this PR. Any chance that's an easy fix? — Reply to this email directly, view it on GitHub<#1571 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ADLSM5XRGQN4N6DMWX7CGODUVRVAJANCNFSM5LLA6XKA>. Triage notifications on the go with GitHub Mobile for iOS<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

sbailey · 2022-01-11T18:59:00Z

@schlafly I ran the missing zmtl files and updated the night qa pages so that they now have the per-petal n(z) plots.

schlafly · 2022-01-11T19:08:46Z

Great. This makes clear that this branch fixes the r1 issue and repairs the redshift success rate.

(most prominent as dips in r1 redshift success rate on LRG, ELG, QSO targets, that go away in the new reduction)
However, the redshift success rate also drops by ~half a percent everywhere on all petals and all target classes, maybe due to the visibly increased noise in the sframe files, which I don't understand at all?

ashleyjross · 2022-01-11T20:02:10Z

I think here again it requires using only the exact same data in the comparison. I do so using this script (which I think can be used pretty generally for comparisons going forward): https://github.com/desihub/LSS/blob/master/Sandbox/comp_newspecwdaily.py I find that for LRGs, the success rate (using Rongpu's criteria) is exactly the same in all petals except for petal 1. In petal 1, it improves 1.4% (from 97.0 to 98.4, making it typical of the other petals). I think we did indeed expect nothing to have changed at all except for in petal 1? From: Eddie Schlafly ***@***.***> Date: Tuesday, January 11, 2022 at 2:08 PM To: desihub/desispec ***@***.***> Cc: ashleyjross ***@***.***>, Comment ***@***.***> Subject: Re: [desihub/desispec] Fit offset per CCD sector as part of sky subtraction (PR #1571) Great. This makes clear that this branch fixes the r1 issue and repairs the redshift success rate. [image]<https://user-images.githubusercontent.com/1417841/149005488-65a75ed3-0052-4887-8a06-eec1afb9e606.png> [image]<https://user-images.githubusercontent.com/1417841/149005621-40f4dc75-6466-4845-9c95-b06b9ec52198.png> (most prominent as dips in r1 redshift success rate on LRG, ELG, QSO targets, that go away in the new reduction) However, the redshift success rate also drops by ~half a percent everywhere on all petals and all target classes, maybe due to the visibly increased noise in the sframe files, which I don't understand at all? — Reply to this email directly, view it on GitHub<#1571 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ADLSM5XCWXI4KXP4D6BEHQDUVR54RANCNFSM5LLA6XKA>. Triage notifications on the go with GitHub Mobile for iOS<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you commented.Message ID: ***@***.***>

schlafly · 2022-01-11T20:16:52Z

Yes, good call. And yeah, if I actually blink the tileqa files with one another, they're identical except for P1, so that checks out. I don't understand why the sframe files look like they have pretty different noise levels, but maybe there's been some plotting change?

ashleyjross · 2022-01-11T20:27:58Z

Here's a plot of the results, using deltachi2 > 25 for ELGs (blue) and quasars (yellow) (but still using Rongpu's criteria for the LRGs in red). [Chart, line chart Description automatically generated] From: Eddie Schlafly ***@***.***> Date: Tuesday, January 11, 2022 at 3:17 PM To: desihub/desispec ***@***.***> Cc: ashleyjross ***@***.***>, Comment ***@***.***> Subject: Re: [desihub/desispec] Fit offset per CCD sector as part of sky subtraction (PR #1571) Yes, good call. And yeah, if I actually blink the tileqa files with one another, they're identical except for P1, so that checks out. I don't understand why the sframe files look like they have pretty different noise levels, but maybe there's been some plotting change? — Reply to this email directly, view it on GitHub<#1571 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ADLSM5QMRTR6G7DJIKPWSKLUVSF35ANCNFSM5LLA6XKA>. Triage notifications on the go with GitHub Mobile for iOS<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you commented.Message ID: ***@***.***>

ashleyjross · 2022-01-11T21:36:59Z

Here's a plot of the results, using deltachi2 > 25 for ELGs (blue) and quasars (yellow) (but still using Rongpu's criteria for the LRGs in red). [Chart, line chart Description automatically generated] From: Eddie Schlafly @.> Date: Tuesday, January 11, 2022 at 3:17 PM To: desihub/desispec @.> Cc: ashleyjross @.>, Comment @.> Subject: Re: [desihub/desispec] Fit offset per CCD sector as part of sky subtraction (PR #1571) Yes, good call. And yeah, if I actually blink the tileqa files with one another, they're identical except for P1, so that checks out. I don't understand why the sframe files look like they have pretty different noise levels, but maybe there's been some plotting change? — Reply to this email directly, view it on GitHub<#1571 (comment)>, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ADLSM5QMRTR6G7DJIKPWSKLUVSF35ANCNFSM5LLA6XKA. Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub. You are receiving this because you commented.Message ID: @.***>

Plot didn't come through email

:

julienguy · 2022-01-11T22:22:30Z

@schlafly I have plotted a few sframe spectra for the 2 prods and they are exactly the same for all cameras except for r1 of course where there is a tiny offset. So this change in the figure is due to the display not the data. And I don't know what was changed in the display.

araichoor · 2022-01-11T22:50:04Z

I agree it's weird.
I don't see an obvious reason in the code.
I confirm I spot-checked exposures/20211222/00115150/sframe-b0-00115150.fits, and the FLUX are exactly the same for the skycte2 and daily prods.

Though I notice that the sframesky-20211222.pdf file size is different for the two prods:

-rw-r----- 1 desi desi 11M Dec 23 19:35 /global/cfs/cdirs/desi/spectro/redux/daily/nightqa/20211222/sframesky-20211222.pdf
-rw-rw---- 1 sjbailey desi 6.9M Jan 11 10:53 /global/cfs/cdirs/desi/users/sjbailey/spectro/redux/skycte2/nightqa/20211222/sframesky-20211222.pdf

with the skycte2 one being "lighter" than the daily one (skycte has one page less, 10 instead of 11, but doesn t explain why it s ~twice lighter).
=> could that explain that skycte2 is "noisier"? ie maybe the plot is at a lower resolution?

looking at the file characteristics, I see that daily used matplotlib pdf backend 3.2.1 and skycte2 used Matplotlib pdf backend v3.5.1
don t know if that helps... have to go offline now.

sbailey · 2022-01-11T22:53:30Z

Thanks for all the checks. This looks good as long as the corrections remain "opt-in" for specific fibers on specific date ranges.

I'm not going to worry about the plotting grayscale differences. As Anand noted, this could arise from the different matplotlib version (old desiconda vs. new desiconda).

araichoor · 2022-01-12T00:24:11Z

Just to further confirm it likely is a display issue, I ran the script for one exposure with both prods, and the only visible change when flipping between the two pdf is for r1, as expected.

from desispec.night_qa import create_sframesky_pdf
create_sframesky_pdf("tmp-daily.pdf", 20211222, "/global/cfs/cdirs/desi/spectro/redux/daily", [115150])
create_sframesky_pdf("tmp-skycte2.pdf", 20211222, "/global/cfs/cdirs/desi/users/sjbailey/spectro/redux/skycte2", [115150])

tmp-daily.pdf
tmp-skycte2.pdf

julienguy added 7 commits December 13, 2021 17:44

not working well

e578b16

fit for sky scale before computing offset

927d0af

mask using ivar

f5fe18a

Merge branch 'master' into bkg-per-amp-skysub

6dcc474

add option to fit a background in sectors of the CCD as part of the s…

c8c39d0

…ky subtraction

account for fiberflat

b013032

forgot to propagate fiberflat

720eeff

docstring fix

ed899fd

cleanup

fce308a

schlafly reviewed Jan 6, 2022

View reviewed changes

remove leftover duplicated code

239aad2

julienguy added 2 commits January 6, 2022 09:40

new approach to fit the offsets

3f2ce21

add back bkg to frame if used afterwards

f7ac942

sbailey merged commit cde451d into master Jan 11, 2022

sbailey deleted the offset-per-sector-skysub branch January 11, 2022 22:53

sbailey changed the title ~~Fit offset per CCD sector as part of sky subtraction~~ CTE fix: fit offset per CCD sector as part of sky subtraction Jan 12, 2022

julienguy mentioned this pull request Aug 17, 2022

Report on CTE software correction #1826

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CTE fix: fit offset per CCD sector as part of sky subtraction #1571

CTE fix: fit offset per CCD sector as part of sky subtraction #1571

julienguy commented Jan 5, 2022

coveralls commented Jan 5, 2022 •

edited

julienguy commented Jan 5, 2022

schlafly left a comment

schlafly Jan 6, 2022

julienguy Jan 6, 2022

sbailey commented Jan 6, 2022 •

edited

julienguy commented Jan 6, 2022

julienguy commented Jan 6, 2022

sbailey commented Jan 6, 2022

sbailey commented Jan 11, 2022

schlafly commented Jan 11, 2022

schlafly commented Jan 11, 2022

ashleyjross commented Jan 11, 2022 via email

sbailey commented Jan 11, 2022

schlafly commented Jan 11, 2022

ashleyjross commented Jan 11, 2022 via email

schlafly commented Jan 11, 2022

ashleyjross commented Jan 11, 2022 via email

ashleyjross commented Jan 11, 2022

julienguy commented Jan 11, 2022

araichoor commented Jan 11, 2022

sbailey commented Jan 11, 2022

araichoor commented Jan 12, 2022

CTE fix: fit offset per CCD sector as part of sky subtraction #1571

CTE fix: fit offset per CCD sector as part of sky subtraction #1571

Conversation

julienguy commented Jan 5, 2022

coveralls commented Jan 5, 2022 • edited

julienguy commented Jan 5, 2022

schlafly left a comment

Choose a reason for hiding this comment

schlafly Jan 6, 2022

Choose a reason for hiding this comment

julienguy Jan 6, 2022

Choose a reason for hiding this comment

sbailey commented Jan 6, 2022 • edited

julienguy commented Jan 6, 2022

julienguy commented Jan 6, 2022

sbailey commented Jan 6, 2022

sbailey commented Jan 11, 2022

schlafly commented Jan 11, 2022

schlafly commented Jan 11, 2022

ashleyjross commented Jan 11, 2022 via email

sbailey commented Jan 11, 2022

schlafly commented Jan 11, 2022

ashleyjross commented Jan 11, 2022 via email

schlafly commented Jan 11, 2022

ashleyjross commented Jan 11, 2022 via email

ashleyjross commented Jan 11, 2022

julienguy commented Jan 11, 2022

araichoor commented Jan 11, 2022

sbailey commented Jan 11, 2022

araichoor commented Jan 12, 2022

coveralls commented Jan 5, 2022 •

edited

sbailey commented Jan 6, 2022 •

edited