Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Effective Focal Length #1223

Closed
kosack opened this issue Feb 12, 2020 · 13 comments
Closed

Effective Focal Length #1223

kosack opened this issue Feb 12, 2020 · 13 comments
Labels
physics science-performance related issues refactoring

Comments

@kosack
Copy link
Contributor

kosack commented Feb 12, 2020

Currently in the transform from CameraFrame (meters on the camera plane) to TelescopeFrame (degrees on the sky, before refraction), we use the nominal focal_length as read from the SimTelarray files (actually we use what we call the "equivalent focal length", which is the nominal focal length taking into account the 1 or 2-mirror optics).

However, the correct focal length to use is the effective focal length measured using an optical point-source ray-traced from a distance. The difference is something like 2% for MSTs, and 4% for LSTs (see table from Konrad Bernloehr). I believe using this focal length in the coordinate transform is equivalent to the "optical aberration correction" used in MARS (@moralejo is that correct? - there it uses the effective/nominal ratio to shrink the camera, but that's the same as just using the effective focal length in the coordinate transform)

Here is how I suggest how to proceed (let me know if there are better ideas):

  1. produce a lookup table (just a text file in ECSV forma) for the effective focal length for each optics type, and put it in ctapipe-extra (so it can be modified, or replaced, for new MCs, or better measurements). It should not be hard-coded
  2. either:
    1. in the SimTelEventSource check for this file, and replace equivalent_focal_length per telescope with the effective one if it exists, requiring no further change to the coordinate transform code.
    2. add a field effective_focal_length to OpticsDescription (and fill it in SimTelEventSource), and then do the check when we do the coordinate transform of which one to pass to the CameraFrame.
@kosack kosack added physics science-performance related issues refactoring labels Feb 12, 2020
@kosack
Copy link
Contributor Author

kosack commented Feb 12, 2020

option 2.ii is somewhat similar to what is in #1222 (but there the decision is done inside CameraFrame, not outside)

@maxnoe
Copy link
Member

maxnoe commented Feb 12, 2020

I have a pretty strong opinion that this should be solved outside the coordinate transforms, similarly to the discussion about pointing directions.

The trafos only need a focal_length and a pointing. It should be the best estimate we have. But which measurement or input datum defines that best estimate is a question of what to give to the transform, not something that is handled inside the transform.

@maxnoe
Copy link
Member

maxnoe commented Feb 12, 2020

For the simtel files, the question is: which focal length is simulated? I guess the the focal length of the single mirrors and the positions are defined and one has to measure or calculate the resulting focal length of the full reflector somehow?

@moralejo
Copy link
Contributor

moralejo commented Feb 12, 2020 via email

@kosack
Copy link
Contributor Author

kosack commented Feb 12, 2020

I have a pretty strong opinion that this should be solved outside the coordinate transforms, similarly to the discussion about pointing directions.

But here we are not changing the coordinate transform, rather we are passing the "correct" focal length to it. It requires no extra attributes to the transform, just when you construct a CameraFrame, you set focal_length=effective_focal_length rather than focal_length=equivalent_focal_length. SO it is outside the transform.

For the pointing corrections, i'm not sure I agree - those are just coordinate transformations (rotation + translation), do belong in the transform. In most astronomical observatories coordinate and "WCS" transforms contain a large series of instrumental transforms - its one of the reasons there is a push to replace FITS with ASDF, since FITS only supports simplistic transformations.

@maxnoe
Copy link
Member

maxnoe commented Feb 12, 2020

But here we are not changing the coordinate transform, rather we are passing the "correct" focal length to it.

Yes, just to reiterate on the discussion in the PR how this should be handled. Sorry, wasn't opposing your comments.

@orelgueta
Copy link
Contributor

Sorry for commenting so late (quarantine provides time to read old issues).

The focal length used during the simulation in sim_telarray is the nominal one. However, the effective focal length is calculated once for each telescope and is then passed as a dummy parameter in order to be available in the sim_telarray output for analysis. The parameter name is effective_focal_length.
The only issue is that it was added after prod3, so for analysing prod3b files, an external table has to be used. For the future it is better to get it from the output, to keep up with any optical model updates.

@kosack
Copy link
Contributor Author

kosack commented Apr 7, 2020

@orelgueta that's good to know! I had no idea it was in the MCs. Then we need to read it when constructing the SubarrayDescription. I'll open a PR

@kosack
Copy link
Contributor Author

kosack commented Apr 7, 2020

Question: there are 2 ways of implementing this:

  1. change OpticsDescription as mentioned above to store both nominal and effective focal lengths, and then add an option (where?) to choose which one to use in the coordinate transform from Camera to TelescopeFrame (not so easy, since there is no way to configure that, at least not until we define some. That means an API change + a new column in the SubarrayDescription output table. So it a largish change.
  2. simply add an option to SimTelEventSource that tells it which one to prefer when filling the focal length in to the subarray (with no change to the subarray or coordinate transforms necessary). In that case, if the preference is "effective", it would use that when available, but fall back to nominal if not.

I think I prefer option 2, since it's simpler. Any reason why we would want to have both loaded? The only disadvantage to 2 is that if you do an analysis starting with DL1 files rather than DL0, you would not be able to choose which one, since the choice would have already been made. However, I think that isn't something we need to do often, just for testing once or twice, so starting from DL0 is ok.

@kosack
Copy link
Contributor Author

kosack commented Apr 7, 2020

Is there a sample file somewhere where the effective_focal_length is included? I guess it would appear in

simtelfile['telescope_descriptions'][tel_id]['camera_settings']['effective_focal_length']

(as read by the eventio python library)?

@maxnoe
Copy link
Member

maxnoe commented Apr 7, 2020

Yes, It's in the eventio test resources:, e.g. this one: pyeventio/tests/resources/prod4_pixelsettings_v3.gz

@kosack
Copy link
Contributor Author

kosack commented Apr 7, 2020

@maxnoe Is that file part of the built package? (i.e. can I use pkg_resources to access it?)

@maxnoe
Copy link
Member

maxnoe commented Apr 7, 2020

I don't think we include the test resources.

$ curl -LO https://github.com/cta-observatory/pyeventio/raw/master/tests/resources/prod4_pixelsettings_v3.gz

@kosack kosack closed this as completed in c96bee4 Apr 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
physics science-performance related issues refactoring
Projects
None yet
Development

No branches or pull requests

4 participants