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

tel_id in photoelectrons object is index rather than id? #281

Open
orelgueta opened this issue Sep 16, 2024 · 1 comment
Open

tel_id in photoelectrons object is index rather than id? #281

orelgueta opened this issue Sep 16, 2024 · 1 comment

Comments

@orelgueta
Copy link
Contributor

Looking at a few events, I think that the telescope id in the Photolectrons object defined below is just the index rather than the telescope id itself. That is, it starts at zero and goes to n_tel - 1.

self.telescope_id = header.id % 1000

This definition is different to the definition in the TelescopeEvent object which provides the actual id:

self.telescope_id = self.type_to_telid(header.type)
self.global_count = header.id
@staticmethod
def type_to_telid(eventio_type):
base = eventio_type - 2200
return 100 * (base // 1000) + base % 1000

Not a big deal, but if I am right and the fix is just replacing one definition with the other, it's probably worth it to fix (I can do it).

@maxnoe
Copy link
Member

maxnoe commented Sep 16, 2024

Looking at a few events, I think that the telescope id in the Photolectrons object defined below is just the index rather than the telescope id itself. That is, it starts at zero and goes to n_tel - 1.

This is a known issue in sim telarray, I will dig up some emails from Konrad, but the gist is, the object is correctly defined here in terms of doing the I/O of the eventio object, but simtel fills something "wrong".

I.e. the header struct defines this to be the telescope_id, which is why we parse it as such, but in case of simtel, it's actually not the same "telescope_id" as used everywhere else.

See e.g.:
https://github.com/cta-observatory/ctapipe/blob/3dc94da64dc1e96ed9316a612c8a6c67ada32af0/src/ctapipe/io/simteleventsource.py#L816-L820

and this issue / PR pair in ctapipe:

cta-observatory/ctapipe#1338
cta-observatory/ctapipe#1337

I'll leave this open here, so when we do eventio 2.0, we can include the fix here

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