DM-42828: Add camera team crosstalk nonlinearity results #523

plazas · 2024-04-12T18:52:27Z

No description provided.

czwa

I think the coeffValid comment is the only thing that needs to be fixed. Do you have an idea of how large/how difficult it would be to shift the things being retrieved from butler into source files in the _data package would be? I would prefer they be there, so this script is portable (and safe against someone accidentally overwriting the collections).

czwa · 2024-04-16T19:53:07Z

python/lsst/obs/lsst/script/write_non_linear_crosstalk_coeffs.py

+
+# PTC in butler query-collections /sdf/group/rubin/repo/main
+#  u/snyder18/*crosstalk_analysis*
+butlerPtc = Butler(repo, collections="u/lsstccs/ptc_13144_w_2023_22/20230607T013806Z")


I assume this is the collection Adam used while making his measurements, but it would be good to have a comment about that, as this is a different run than the two listed below.

czwa · 2024-04-16T19:55:23Z

python/lsst/obs/lsst/script/write_non_linear_crosstalk_coeffs.py

+
+    for i, ref in enumerate(data_refs):
+        crosstalk_results = butler.get(ref)
+        for j in range(nAmp):


Maybe this_ct_result = crosstalk_results[det_name][det_name] so the assignments below can avoid needing to break the keys up over multiple lines?

czwa · 2024-04-16T20:00:49Z

python/lsst/obs/lsst/script/write_non_linear_crosstalk_coeffs.py

+from lsst.ip.isr import CrosstalkCalib
+import lsst.utils
+
+repo = "/sdf/group/rubin/repo/main"


This would be safer if the files were dumped from butler into the obs_lsst_data package, so this isn't location specific.

Commenting here that I'm not worried about this anymore.

czwa · 2024-04-16T20:04:05Z

python/lsst/obs/lsst/script/write_non_linear_crosstalk_coeffs.py

+                    c0_matrix[k, j, i] = 0.0
+                    c1_matrix[k, j, i] = 0.0
+                    continue
+                try:


Why does this fail that we need a try/except block?

From Adam: "when we take the images there are only a subset that have actual crosstalk measurements (since we need to readout at least a REB worth of images). So when I run the analysis there are some empty crosstalk datasets where there was no source detected for that specific amp pair."

czwa · 2024-04-16T20:06:42Z

python/lsst/obs/lsst/script/write_non_linear_crosstalk_coeffs.py

+                if j == k:
+                    c0_matrix[k, j, i] = 0.0
+                    c1_matrix[k, j, i] = 0.0
+                    continue


I don't know that it's essential, but these should set the coeffValid entry to False. If we had made these calibrations with cp_pipe, they would be set that way (as a coefficient of zero should be smaller than its error), and it's good to be consistent.

For the diagonal coefficients (same amp), do we currently set them as invalid?

We should be: https://github.com/lsst/cp_pipe/blob/main/python/lsst/cp/pipe/measureCrosstalk.py#L642

plazas · 2024-04-17T13:48:33Z

I think the coeffValid comment is the only thing that needs to be fixed. Do you have an idea of how large/how difficult it would be to shift the things being retrieved from butler into source files in the _data package would be? I would prefer they be there, so this script is portable (and safe against someone accidentally overwriting the collections).

@czwa I can do this. Do you want two scripts?: one to get the coefficients from Butler and then put them in fits files, and then another one to read those fits files and save them as ecvs? Or do you want this single script to keep doing what it's doing, but saving the coeffs in fits files after being retrieved from the Butler?

timj · 2024-04-17T14:03:52Z

The standard approach is to create the ECSV files and store them in obs_lsst_data and then they become standard curated calibrations that can be loaded into any butler. The usual thing is that the ECSV files are read and then butler.put turns them into FITS to let them be read as fast as possible, but the files are small enough that storing them as text in git makes it easier.

plazas · 2024-04-17T14:07:54Z

The standard approach is to create the ECSV files and store them in obs_lsst_data and then they become standard curated calibrations that can be loaded into any butler. The usual thing is that the ECSV files are read and then butler.put turns them into FITS to let them be read as fast as possible, but the files are small enough that storing them as text in git makes it easier.

Thanks. This script is already getting the coefficients from the Butler and saving them as ecsv files in obs_lsst_data (there's another PR in there with this same ticket number). Is that sufficient in this case for this ticket, or should the butler.put step to turn them into fits files also be implemented?

timj · 2024-04-17T14:13:29Z

If this is a standard IsrCalib class (which it should be) then it should already be storing as FITS in the butler when it's read from obs_lsst_data.

czwa · 2024-04-17T19:26:09Z

python/lsst/obs/lsst/script/write_non_linear_crosstalk_coeffs.py

+        run_num = 13199
+
+    # Crosstalk from A. Snyder (UCDavis, 2024)
+    collections = "u/snyder18/{0}/crosstalk_analysis".format(run_num)


I think I'm fine with leaving the source data in butler, as long as this is changed to explicitly point at the RUN collection (with the full date string at the end, as is done with the PTC collection).

czwa · 2024-04-17T19:30:18Z

My concern was that the source data would be lost, leaving us no way to regenerate these files if needed. I think ensuring that the script uses the RUN collection instead of the CHAINED collection ensures that we can find things. A lot of the other curated calibrations have no clear provenance, so I've pushed for the sources of those products to be included in the _data package. I think we're safer with things in Butler.

timj · 2024-04-17T19:33:35Z

So long as write-curated-calibrations is using obs_lsst_data and not trying to find a specific butler that's fine. I assume there is a readme in obs_lsst_data that says where the data came from. ie the scripts that created the ECSV files from butler are part of the provenance but not something used regularly.

czwa · 2024-04-17T19:49:55Z

So long as write-curated-calibrations is using obs_lsst_data and not trying to find a specific butler that's fine. I assume there is a readme in obs_lsst_data that says where the data came from. ie the scripts that created the ECSV files from butler are part of the provenance but not something used regularly.

Yes, write-curated-calibrations is doing the standard thing (pull from obs_lsst_data, and write to the target butler). My concern was prompted by things like the transmission curves being based on an xls spreadsheet (where that one file and some email quotes are the only provenance we have). Pointing at the butler RUN collections should be stable and reproducible, so I think my concerns were inflated.

czwa · 2024-04-25T23:17:41Z

python/lsst/obs/lsst/script/write_non_linear_crosstalk_coeffs.py

+from lsst.ip.isr import CrosstalkCalib
+import lsst.utils
+
+repo = "/sdf/group/rubin/repo/main"


Commenting here that I'm not worried about this anymore.

Fix typo Reformat comment

plazas requested a review from czwa April 12, 2024 18:52

czwa reviewed Apr 16, 2024

View reviewed changes

czwa reviewed Apr 17, 2024

View reviewed changes

czwa approved these changes Apr 25, 2024

View reviewed changes

plazas force-pushed the tickets/DM-42828 branch from 8d7fba8 to fa50f10 Compare April 26, 2024 14:51

plazas added 6 commits April 29, 2024 10:58

Add script to write crosstalk coeffs c0 and c1

005074d

Run Black to fix linter errors

f430078

Add comment about collections

caf5c32

Use temp variable to avoid multiple lines

7487f0f

Use run collections

ec6027e

Add the coeffsValid matrix

4dfac2b

Fix typo Reformat comment

plazas force-pushed the tickets/DM-42828 branch from fa50f10 to 4dfac2b Compare April 29, 2024 17:59

plazas merged commit 5a4ba4b into main Apr 29, 2024
3 checks passed

plazas deleted the tickets/DM-42828 branch April 29, 2024 18:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DM-42828: Add camera team crosstalk nonlinearity results #523

DM-42828: Add camera team crosstalk nonlinearity results #523

plazas commented Apr 12, 2024

czwa left a comment

czwa Apr 16, 2024

czwa Apr 16, 2024

czwa Apr 16, 2024

czwa Apr 25, 2024

czwa Apr 16, 2024

plazas Apr 18, 2024

czwa Apr 16, 2024

plazas Apr 17, 2024

czwa Apr 17, 2024

plazas commented Apr 17, 2024

timj commented Apr 17, 2024

plazas commented Apr 17, 2024

timj commented Apr 17, 2024

czwa Apr 17, 2024

czwa commented Apr 17, 2024

timj commented Apr 17, 2024 •

edited

czwa commented Apr 17, 2024

czwa Apr 25, 2024

DM-42828: Add camera team crosstalk nonlinearity results #523

DM-42828: Add camera team crosstalk nonlinearity results #523

Conversation

plazas commented Apr 12, 2024

czwa 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

plazas commented Apr 17, 2024

timj commented Apr 17, 2024

plazas commented Apr 17, 2024

timj commented Apr 17, 2024

Choose a reason for hiding this comment

czwa commented Apr 17, 2024

timj commented Apr 17, 2024 • edited

czwa commented Apr 17, 2024

Choose a reason for hiding this comment

timj commented Apr 17, 2024 •

edited