LSSTDESC / imSim

GalSim based Rubin Observatory image simulation package
https://lsstdesc.org/imSim
BSD 3-Clause "New" or "Revised" License
36 stars 15 forks source link

Put currently measured sensor properties into the obs_lsstCam policy files. #130

Closed cwwalter closed 5 years ago

cwwalter commented 6 years ago

As part of the move from obs_lsstSim to obs_lsstCam (tracked in #121) we want to take I&T measured sensor properties, X-talk, noise etc etc. and put them into the obs_lsstCam policy yaml files being produced by @RobertLuptonTheGood and @mfisherlevine etc.

This is a nice small sensor related student project!

We will need a good explanation of the format and entries from the obs_lsstCam team.

cwwalter commented 6 years ago

@snyder18 has taken the the latest SLAC sensor x-talk measurements and written them in the YAML form needed for the obs_lsstCam camera objects. These objects are used by both imSim and the stack.

I made this issue for updating information such as the x-talk parameters. Adam can you attach them here? If you could also write down what sensor they are from etc that would be good. You can drag and drop the two files to a window. Then @rhl and @jchiang87 can take a look.

Snyder005 commented 6 years ago

RTM-006_E2V-CCD250-131_S11_crosstalk.txt RTM-011_ITL-3800C-136_S11_crosstalk.txt

It appears that Github does not support the YAML file type for uploads, so I had to change the extension to TXT, however this shouldn't have affected the formatting, and you should only need to change it back to YAML.

cwwalter commented 6 years ago

Thanks @snyder18 ! @rhl and @jchiang87 could you confirm these are OK?

jchiang87 commented 6 years ago

I was able to add the ITL values to obs_lsstCam/policy/imsim/ccdData.yaml and generate the policy/imsim.yaml file via scons. The coefficients are returned from the detector objects provided by the ImsimMapper().camera object, so I think these are good to go.

cwwalter commented 6 years ago

Thanks @jchiang87.

@smr456 could you sign off on these numbers being good to use?

@snyder18 After we are using this would you be willing to add a bit of documentation here:

https://github.com/LSSTDESC/imSim/wiki/Feature-Matrix

If I send you instructions?

cwwalter commented 6 years ago

Also, @smr456, do you think we can store the files in this format in the CCB controlled throughputs repository?

cwwalter commented 6 years ago

@smr456. Can you sign off that these numbers are OK to use for starting the 2.0i run?

Thanks.

-Chris

cwwalter commented 6 years ago

@jchiang87 Are these numbers in a version the yaml policy files in obs_lsstCam somewhere already?

jchiang87 commented 6 years ago

Not yet. I was going to make a PR today or tomorrow.

jchiang87 commented 6 years ago

Here's the PR: https://github.com/lsst/obs_lsstCam/pull/18

smr456 commented 6 years ago

I get a 404 error when I click on the feature matrix link. No matter: I looked at the xtalk .txt files and they look OK for now. Of course, these will update as we do more raft and integrated focal plane testing. I will put these numbers in the SE github when I return from vacation next week, but there is no need for that to hold you up. Does that give you what you need from me now? Again, what matters most here is the performance after DM signature removal and correction, so it is essential to coordinate closely with DM.

cwwalter commented 6 years ago

Does that give you what you need from me now?

Yes, thank you Steve. All: We can go ahead with these x-talk numbers now.

@jchiang87 what are we doing for read noise and dark current? Do we need anything new/approved?

I believe we are using constant values across amps now correct?

cwwalter commented 6 years ago

@snyder18 Once Steve gets the numbers in the official SE repo, are you up for making a validation/explanation page on the imSim wiki for the x-talk values?

jchiang87 commented 6 years ago

Some of those parameters are set here: https://github.com/LSSTDESC/imSim/blob/master/data/default_imsim_configs

In the obs_lsstCam policy files, the gains are all 1.7 and the read noise levels are 7., e.g.,

https://github.com/lsst/obs_lsstCam/blob/u/jchiang/imsim_crosstalk/policy/lsstCam/R01.yaml

I think the gains should be changed to 0.7 to match current testing. read noise of 7 seems fine.

cwwalter commented 6 years ago

OK, thanks Jim.

In the obs_lsstCam policy files, the gains are all 1.7 and the read noise levels are 7., e.g.,

https://github.com/lsst/obs_lsstCam/blob/u/jchiang/imsim_crosstalk/policy/lsstCam/R01.yaml

I think the gains should be changed to 0.7 to match current testing. read noise of 7 seems fine.

I think I saw some numbers with a somewhat higher median noise but that was a while ago. @smr456 do you have any input on these also?

Specifically we want to use:

for all sensors as a reasonable performance characteristic now.

I believe these are the only other numbers we now need to check off on.

mfisherlevine commented 6 years ago

Not really a comment on the values, but if it were me, I'd make them all 0.7gain / 7e noise or whatever nominally but with some variation from amp-to-amp. I would make sure that they were NOT stochastic (i.e. hard-coded values for each), but I wouldn't have them all identically valued either, much too easy for some potential underlying problems to get glossed over like that. Just my 2¢ though.

cwwalter commented 6 years ago

Here's the PR: lsst/obs_lsstCam#18

This wasn't merged. Does that mean that Adam's #s aren't in the package that got built for the release candidate?

jchiang87 commented 6 years ago

I made tag of that branch. That's what's in the rc distribution.

cwwalter commented 5 years ago

@smr456 For run 2.1i we want to put updated gain and read noise into the simulation. The x-talk parameters were approved, but these were never signed off on. We would like to use the values:

Can you confirm these are still OK for us so we can merge them into the lsstCam description? Currently, we have old values there (i.e. 1.7 ADU/electron for the gain).

Thanks.

jchiang87 commented 5 years ago

Here is the PR for this in obs_lsst: https://github.com/lsst/obs_lsst/pull/41

cwwalter commented 5 years ago

@smr456 Pinging you again on this issue.

smr456 commented 5 years ago

Sorry, I was in transit, and for some reason did not see the earlier notice. I will look today. Steve

On Nov 26, 2018, at 19:55, Chris Walter notifications@github.com wrote:

@smr456 https://github.com/smr456 Pinging you again on this issue.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/LSSTDESC/imSim/issues/130#issuecomment-441895114, or mute the thread https://github.com/notifications/unsubscribe-auth/AMRHAM23fBQ1nvkBrEPn_MRgbRrShNAGks5uzLe2gaJpZM4V0qdj.

smr456 commented 5 years ago

It really depends on how you want to use the simulation. If you are asking if the gain and noise are sensible, the answer is yes. However, as we have discussed and you all know well, what really matters to DESC is what the residual effects are after ISR, so that requires both the effect and the (imperfect) removal. For gain, the closely related issues are non-linearities, saturation, and variations, but if you just want a number and you want to assume these effects are handled perfectly for now, fine. For noise, a median of 7e is good. Merlin is right that we expect significant variation across the focal plane. I reported this in a SAWG early this year. I don't agree that these should be "stochastic" in the sense that we expect that to be stable for stable operating conditions. You could put in a random spatial distribution of 50% of the sensors at 7e (many will be better, but it won't matter too much), 25% at 9e, 20% at 13e, and 5% at 18e. This is covered in LPM-262. Does that answer your question, @cwwalter ?

cwwalter commented 5 years ago

Yes, thank you very much Steve.

Saturation is handled separately and your other issues are understood. We may handle gain correction through the measured DM gains (which are imperfect) or eventually introduce variance in the given correction numbers to give us imperfect removal. For this simulation we would like to keep the read noise constant from chip to chip. We have made enough other things complex and varying that we would like to wait on making these parameters vary on a per sensor basis to the next round of simulations.

I'm closing this issue now as all of the relevant sensor numbers have now been approved by the camera team.

We should merge lsst/obs_lsst#41 (where these values have been entered) but I am not sure of the exact procedure given that the package transition and control.