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

Refactor/improve definition and use of TelescopeDescription and its string represenation #2675

Open
maxnoe opened this issue Jan 9, 2025 · 2 comments

Comments

@maxnoe
Copy link
Member

maxnoe commented Jan 9, 2025

This also makes me wonder why we have the same string rep for different types in the first place, but good to fix. Really, the telescope type string was supposed to have three parts:

  • a high-level type name: LST/MST/SST
  • a optics name
  • a camera name
    When anything changes in the description, the optics or camera name should also be changed, i,e by adding a version, but we don't enforce that. The optics name should not in general be just "LST" (which is redundant), but rather something version-able or that describes better the current optics design. I would expect in the future we could have e.g. 1: LST_v1_LSTCam, 2,3,4: LST_v2_LSTCam, but that needs to be done at the simualtion production level when setting the metadata.

Originally posted by @kosack in #2673 (review)

@maxnoe
Copy link
Member Author

maxnoe commented Jan 9, 2025

This came up multiple times, but we don't seem to have an issue for it, at least a quick search doesn't bring on up.

To comment, we have multiple conflicting wanted behaviors with respect to telescope descriptions and their string versions.

Some observations

  1. TelescopeDescription compares and hashes equal only if both the camera and the optics compare equal. This includes many pieces of information. I'll call this the data
  2. The string is constructed from optics.size_type, optics.name and camera.name, I'll call this the string

This leads to e.g. same string but different types in Prod6 for the two LSTs. Originally, this affected only inofficial LST + MAGIC simulations, and we change the assumption that same string means same data to support these simulations, see #1424 and 1426. But now this is even the case in official Prod6 simulations.

In the case of the LSTs, this is due to differences in the camera (readout) definition:

In [8]: s.tel[1].camera.readout == s.tel[2].camera.readout
Out[8]: False

particularly the single pe pulse shape.

We use the string mostly to say "This is sufficiently similar to treat it as the same telescope type in this context". But I think we just need to let go of this concept and be explicit when we want to group together which telescopes and how.

E.g. we use it to train models or load events for the same "type".

@kosack
Copy link
Contributor

kosack commented Jan 15, 2025

We use the string mostly to say "This is sufficiently similar to treat it as the same telescope type in this context". But I think we just need to let go of this concept and be explicit when we want to group together which telescopes and how.

It's also when viewing a plot of the array, it's nice to have a human-readable version that shows what was done, but it's true that that doesn't need to exactly match the training division. In any case, as long as we provide the metadata we want when generating the simulations, that is solvable. So the question is more for grouping for training, if we need a better interface or not. Either it could be very explicit, like a list of telescopes, but that is a bit annoying to have to specify in the configuration. We could also just make a nicer grouping function that returns the actually different groups by comparing the OpticsDescription and CameraDescriptions correctly, and just do something like append an index if two have the same string rep (e.g. it would return a set of groups and names like: LST_LST_LSTCam_1: [1,], and LST_LST_LSTCam_2: [2,3,4] in the Prod6 files)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants