LSSTDESC / SSim_DC1

Configuration, production, validation specifications and tools for the DC1 Data Set.
4 stars 2 forks source link

Differing coordinate system conventions in CatSim and PhoSim #36

Closed danielsf closed 6 years ago

danielsf commented 7 years ago

DC1 data generation has recently been hampered by an apparent difference in convention between how CatSim converts RA, Dec into coordinates on the LSST focal plane and how PhoSim makes the same conversion. Specifically, to maximize computational efficiency, we used CatSim to find a list of individual chips that were in the DC1 region on the sky after a series of random rotational dithers had been applied to the minion_1016 OpSim observing cadence. Because PhoSim and CatSim disagree on what a camera rotation of +90 degrees means, chips which CatSim declared to be inside the DC1 region were simulated by PhoSim outside of the DC1 region. Below, I will try to quantify the differences between how CatSim converts from RA, Dec to focal plane position and how PhoSim converts from RA, Dec to focal plane position. The two conversions adopt different sign conventions both in terms of the camera rotator angle and in how displacement to the East on the sky corresponds to displacement on the focal plane. I have not found a definitive document allowing me to declare which convention is "correct."

It may be useful to compare this discussion to the discussion here, where the handedness of the PhoSim WCS is shown to be opposite that of the WCS used by ImSim.

https://github.com/LSSTDESC/imSim/issues/39

Some references

A confluence page describing the LSST focal plane

https://confluence.lsstcorp.org/display/LSWUG/Representation+of+a+Camera

The relevant diagram is here:

confluence_image

Official camera team diagrams of the focal plane can be found in DocuShare under LCA-13381. Here is a screenshot of one of the more salient diagrams

camera_team_diagram

I believe these diagrams are consistent with each other. Note that in both R:3,4 is in the +x direction from R:1,4 and R:1,4 is in the +y direction from R:1,0

A test

I generated a series of 12 InstanceCatalogs, each containing only one source. In the first 4 catalogs, I set rotSkyPos=0 and used CatSim's methods to place the object at the center of a specific raft. I then simulated the image corresponding to each catalog with PhoSim. Below, I present the chip that CatSim thought the object was on, the chip that PhoSim simulated the object on, the focal plane coordinates reported by CatSim, and the RA, Dec of the object. In all cases, the telescope is pointed at RA=33.93, Dec=-6.7

CatSim chip PhoSim chip Focal plane x (CatSim; mm) Focal plane y (CatSim; mm) RA Dec
R:1,4 S:1,1 R:3,4 S:1,1 -127 254 34.64 -5.30
R:3,4 S:1,1 R:1,4 S:1,1 127 254 33.22 -5.30
R:1,0 S:1,1 R:3,0 S:1,1 -127 -254 34.64 -8.12
R:3,0 S:1,1 R:1,0 S:1,1 127 -254 33.22 -8.12

Things to note:

For the other 8 catalogs, I used the same 4 RA, Dec positions of the objects, but changed rotSkyPos to see how the objects moved across the focal plane. Below I will report the value of rotSkyPos, the original rotSkyPos==0 CatSim chip, the rotSkyPos!=0 CatSim chip, the original PhoSim chip, the rotSkyPos!=0 chip, and the focal plane coordinates reported by CatSim with rotSkyPos!=0 (again, in mm)

rotSkyPos CatSim rot==0 CatSim rot!=0 PhoSim rot==0 PhoSim rot!=0 Focal x Focal y
45.0 R:1,4 S:1,1 R:0,3 S:1,1 R:3,4 S:1,1 R:4,3 S:1,0 -269 90
45.0 R:3,4 S:1,1 R:1,4 S:2,1 R:1,4 S:1,1 R:3,4 S:0,1 -90 270
45.0 R:1,0 S:1,1 R:3,0 S:0,1 R:3,0 S:1,1 R:1,0 S:2,1 90 -270
45.0 R:3,0 S:1,1 R:4,1 S:1,2 R:1,0 S:1,1 R:0,1 S:1,2 270 -90
90.0 R:1,4 S:1,1 R:0,1 S:1,1 R:3,4 S:1,1 R:4,1 S:1,1 -254 -127
90.0 R:3,4 S:1,1 R:0,3 S:1,1 R:1,4 S:1,1 R:4,3 S:1,1 -254 127
90.0 R:1,0 S:1,1 R:4,1 S:1,1 R:3,0 S:1,1 R:0,1 S:1,1 254 -127
90.0 R:3,0 S:1,1 R:4,3 S:1,1 R:1,0 S:1,1 R:0,3 S:1,1 254 127

Comparing to this image from the Confluence page referenced above

confluence_image

we see the following

The CatSim documentation for the meaning of rotSkyPos can be found here

https://github.com/lsst/sims_utils/blob/master/python/lsst/sims/utils/ObservationMetaData.py#L74

It can be summarized

rotSkyPos North in focal plane East in focal plane
0.0 +y -x
90.0 -x -y
180.0 -y +x
-90.0 +x +y

This is consistent with objects rotating clockwise in the focal plane as rotSkyPos increases.

danielsf commented 7 years ago

Just to make absolutely sure that there are two issues here (the sign convention on rotSkyPos and which direction on the focal plane corresponds to East on the sky), I did a second set of catalogs varying rotSkyPos, dRA, and dDec, where dRA and dDec are the displacement of the astrophysical sources from the center of the field of view.

rotSkyPos dRA dDec CatSim chip PhoSim chip
0.0 0.0 0.7 R:2,3 S:1,1 R:2,3 S:1,1
0.0 0.0 -0.7 R:2,1 S:1,1 R:2,1 S:1,1
0.0 0.7 0.0 R:1,2 S:1,1 R:3,2 S:1,1
0.0 -0.7 0.0 R:3,2 S:1,1 R:1,2 S:1,1
45.0 0.0 0.7 R:1,3 S:2,0 R:3,3 S:0,0
45.0 0.0 -0.7 R:3,1 S:0,2 R:1,1 S:2,2
45.0 0.7 0.0 R:1,1 S:2,2 R:3,1 S:0,2
45.0 -0.7 0.0 R:3,3 S:0,0 R:1,3 S:2,0
90.0 0.0 0.7 R:1,2 S:1,1 R:3,2 S:1,1
90.0 0.0 -0.7 R:3,2 S:1,1 R:1,2 S:1,1
90.0 0.7 0.0 R:2,1 S:1,1 R:2,1 S:1,1
90.0 -0.7 0.0 R:2,3 S:1,1 R:2,3 S:1,1

It does appear that there are two sign conventions to be reckoned with.

danielsf commented 7 years ago

Simon has pointed out that LCA-13381 labels its diagram such that the +z axis points through the CCDs towards the electronics (i.e. up from M1M3). That would mean that CatSim is the one that is mistaken: when North is in the +y direction, East should be in the +x direction (not the -x direction as is currently implemented).

This would resolve the handedness mismatch between CatSim and PhoSim. It does not resolve the question of how rotSkyPos is defined.

danielsf commented 7 years ago

@johnrpeterson Could you please comment on the definition of rotSkyPos in PhoSim (i.e. does it increase as the sky rotates from North towards East or from North towards West)?

danielsf commented 7 years ago

Zeljko has posted in Slack that the official project position is to define the angle as being positive from North towards East. This is consistent with Kem Cook's guess that, in OpSim, when rotSkyPos=90, West is "up" on the focal plane.

cwwalter commented 7 years ago

Let's just be very explicit to avoid future confusion. Which convention currently matches the project definition, CatSim or PhoSim?

danielsf commented 7 years ago

PhoSim agrees with the project convention.

Regardless, I think (and Andy agrees with me) that we should just make CatSim agree with PhoSim since this specific use case (carefully picking which detectors are in an area of interest and which are not) is the only case in which a disagreement of this type can harm us.

egawiser commented 7 years ago

We should be completely explicit about the Project convention and make sure that both pieces of software match it. This requires definining "up" in terms of focal plane coordinates and stating which direction is "up" when RotSkyPos=0, 90, 180. (LSST images should look like the sky, meaning that if North is "up" then East is "to the left" in an image, yes?) Otherwise this could come back to bite us years down the road.

danielsf commented 7 years ago

@egawiser The camera team has defined +x and +y on the camera such that, when North is +y, East is +x. If you look at the second focal plane diagram I posted (the one that almost looks like an engineering diagram), you can see that +z is into the screen. The figure is labeled as "looking through L3 Lens." This means that you are lying on M1M3, looking up at the focal plane. From that perspective, when North is "up", East is "to the left." However, because +z is pointing from M1M3 towards the focal plane,that means that North="up"=+y and East="to the left"=+x. The diagram I am describing is in LCA-13381.

drphilmarshall commented 7 years ago

Nice! @danielsf, all: I assume this will generate a CatSim issue? Please do post it's URL here so we can follow along - I love it when we generate Project tickets :-)

cwwalter commented 7 years ago

What do we need to change on the imSim or lsst-galsiminterface side if we want to change this in the future and the input catalog rotation sense changes?

danielsf commented 7 years ago

@cwwalter I suspect that changing CatSim will fix ImSim automatically, since sims_GalSimInterface depends on CatSim to generate its WCS.

@drphilmarshall the relevant JIRA ticket is here

https://jira.lsstcorp.org/browse/SIM-2398

I will post a link to the actual branch when I start implementing it

danielsf commented 7 years ago

Branches fixing this are under development:

https://github.com/lsst/sims_utils/tree/u/danielsf/fix_rotskypos

https://github.com/lsst/sims_coordUtils/tree/u/danielsf/fix_rotskypos

cwwalter commented 7 years ago

I think we want to be very careful and deliberate about when to merge these.

This will affect the output of the imSim runs so (after confirming that is all that is needed to make imSim follow this convention), we should cut over at a very defined time so we don't accidentally get files with old and new rotational conventions floating around.

danielsf commented 7 years ago

I agree with Chris' concern. I would like to add one of my own, however. There is already at least one project member who is being bit by the CatSim-vs-PhoSim disagreement. I have shipped him a branch of the code to work off of that fixes the CatSim cameramodel. I would like to avoid being stuck in this Schroedinger's-coordinate-system state for too long, though. All of which is to say: I vote for getting this merged as quickly as possible, since it is more than just DESC relying on our simulations code, and I don't know how effective the grapevine will be at disseminating this present discussion.

danielsf commented 7 years ago

I looked at Jim's code in ImSim and imsim_deep_pipeline. All of the code depends on the CatSim (RA, Dec)-to-pixel mapping, so if I fix CatSim, I will fix ImSim. Given that DM is getting ready to merge an API change to their camera model, I am going to start the process of reviewing and merging the fix for this issue.

danielsf commented 7 years ago

For the sake of having this written down: the problem, as always, is worse than we think.

The DM and Camera teams use different coordinate systems. The documentation for afwCameraGeom says that "+x is in the direction of the serial readout". LCA-13381 indicates that the Camera team defines +y as in the direction of the serial readout. Comparing the position of objects in PhoSim with the results of the DM methods that convert between pupil coordinates and pixel coordinates, I believe that the relationship between the coordinate systems is:

Camera +y = DM +x Camera +x = DM -y Camera +z = DM +z

PhoSim generates a WCS that is consistent with the Camera team's coordinate system. I will make sure that sims_GalSimInterface (and, thus, ImSim) does the same.

I will, of course, generate some test images and make sure that the PhoSim and ImSim images look the same (modulo the fact that PhoSim appears to define position angle as being positive towards West, rather than East).

danielsf commented 7 years ago

I have issued pull requests to address this issue

https://github.com/lsst/sims_GalSimInterface/pull/32 https://github.com/lsst/sims_catUtils/pull/88 https://github.com/lsst/sims_coordUtils/pull/18 https://github.com/lsst/sims_utils/pull/30

cwwalter commented 7 years ago

For the sake of having this written down: the problem, as always, is worse than we think.

Thanks for this Scott!

BTW, since I don't know the background of this: Is everyone OK with these inconsistent coordinate systems? I mean is this the plan that was agreed to and everyone understands and wants to live with forever?

danielsf commented 7 years ago

I don't really know the history of this, either (I end up having to pester Simon every 6 months or so about this). My impressions is that people aren't happy but that the die has been cast.

The take-away from my most recent discussion with Simon about this was that it is worthwhile to have ImSim use the Camera team coordinate system, since that is the coordinate system used in PhoSim e-images, and we have already developed the code necessary to process e-images. Using the DM coordinate system (even though this is the coordinate system used on individual amp images) would involve a different processing pipeline for ImSim and PhoSim e-images. The fix I have put forward in the above pull requests is flexible enough that, if we ever want to use the DM coordinate system, we can just by replacing LSSTCameraMapper() with GalSimCameraMapper(lsst_camera()).

cwwalter commented 7 years ago

OK, thanks. I'm not sure who is in charge of this interface. I think it is probably worth just making sure everyone says they are aware of this at a high enough level while we could still conceivably change this. @connolly Who should be pinged?

fjaviersanchez commented 6 years ago

@danielsf @cwwalter I think this is solved for DC2, right?

cwwalter commented 6 years ago

Yes. I think so. There are still constant discussions about the difference in conventions (see the eye scanning room yesterday for example) but I think we can close this issue.