-
Notifications
You must be signed in to change notification settings - Fork 272
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
SubarrayDescription optics filled incorrectly in SimTelEventSource #1057
Comments
My bad, I assumed |
It seems there is no information about the number of reflectors in the simtel files. |
The information is in the files (but I don't know how to read it...): $HESSIOSYS/bin/read_hess -s -v gamma_20deg_0deg_run94___cta-prod4-sst-astri_desert-2150m--sst-astri.simtel.gz | grep -i class
.... MIRROR_CLASS is the parameter describing the different reflectors. |
Another question regarding the name: what is the equivalent focal length? We traditionally distinguish between 'focal length' and 'effective focal length' and I would vote for not changing standing expressions. |
I checked again. This Information is not written into any eventio data structure. It might be in the History objects, but this would require starting to do string parsing of these, something we are not doing at this point and which will very likely be very error prone. |
Looking into this, I would say it's ocmpletely impossible to associate the config lines in the history with telescope ids. At least for the merged gamma test file. |
I suggest to approach Konrad to add this information for the next version of sim_telarray so that this written to eventio for the next production (which very likely will have only one telescope SST type anyway). |
It's the "focal length" that would work if a 2-mirror telescope was a 1-mirror telescope (so such that a 2-mirror is equivalent to a 1-mirror in terms of reconstruction transforms). We originally named it "effective", but then there was another definition for that, namely the real focal length of where the camera is measured, rather than the correct optical focal length (e.g. if the camera sits a bit behind or in front of the true focal position). If there are more standard definitions for these, we can of course use better terms. In any case, we do need to add both the true equivalent focal length and measured equivalent focal length (the measured one is currently missing), and we might want to also give the focal length of both mirrors in the 2-tel case (Even if they aren't used in reconstruction) |
I like the idea of renaming |
For the mirror-class, we can just do a mapping as we do for some other quantities for now until it's in the simtel files. |
Ok, so this sounds like there should be a new eventio container added, e.g.
More?
A telescope / optics / camera name would also be nice to have. |
Ok - I don't see a necessity to have a 'focal length' for single mirror and an 'equivalent focal length' for the dual mirror telescopes if they are used in the same way. 'effective focal' length is indeed the focal length describing the effect of the asymmetric image abberations to the image plate scale and is the one to be used to convert from length to angle in camera coordinates.
|
Just for completeness, the parameter mirror_class in sim_telarray is defined as:
I am not saying that this should be used in ctapipe. Using mirror_class=1 for a single-dish telescope is much more logical! |
Right now the SubarrayDescription filled by SimTelEventSource looks like this, where
num_mirrors
andnum_mirror_facets
is the same:The idea of
num_mirrors
was to distinguish between dual-mirror and single-mirror telescopes, so it should be 1 or 2, depending on the telescope optics type. The alternative would be to change this to something like a string describing the optics, but we removed that since it wasn't always clear (e.g. the LST is not really a Davies-Cotton mirror). That is needed since in some cases we will have to deal with 1 and 2-mirror telescopes differently (e.g. muons)I'm not sure
num_mirror_facets
is even very useful (since it means nothing in the dual-mirror case), and we don't ever use it in the reconstruction, so we could remove it, or addnum_primary_mirror_facets
andnum_secondary_mirror_facets
instead.Eventually we can expand
OpticsDescription
to have all the mirrors explicitly described with their position and orientation, similar toCameraGeometry
, but that can be later - it would be useful to use for the MC configuration, and maybe even for the Muon calculations, but not strictly necessary)The text was updated successfully, but these errors were encountered: