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

Remove averaging of positions in MCs. #206

Merged
merged 1 commit into from
Feb 19, 2024
Merged

Conversation

aleberti
Copy link
Collaborator

Closes #135

@aleberti aleberti added the maintenance Maintenance related label Feb 17, 2024
Copy link
Collaborator

@jsitarek jsitarek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the code was also pruning the subarray of the telescopes that are not in use (i.e. have assigned id of 0). For the MAGIC+LST1 and MAGIC-only I think this is fine because those subarrays are already created as they shoud.
But for the simulations with multiple LSTs, or for MAGIC-mono case (i.e. e.g. M2+LST-1 observations), the code I think will introduce changes w.r.t. previous behaviour. Might be useful if @ranieremenezes and @Elisa-Visentin have a look as well.

@aleberti
Copy link
Collaborator Author

@jsitarek This is still done. The relevant part is:

event_source = EventSource(
        input_file,
        allowed_tels=list(
            filter(lambda check_id: check_id > 0, assigned_tel_ids.values())
        ),  # Here we load the events for all telescopes with ID > 0.
        focal_length_choice=focal_length,
    )

by passing allowed_tels, internally the SimTelEventSource will select the telescopes from the subarray, and the lambda function is already taking away those with telescope ID set to. See https://github.com/cta-observatory/ctapipe/blob/main/src/ctapipe/io/simteleventsource.py#L696-L697

@jsitarek
Copy link
Collaborator

ok, very good, thanks for checking

@aleberti aleberti merged commit 3cad717 into master Feb 19, 2024
6 checks passed
@aleberti aleberti deleted the remove_average_positions_mc branch February 19, 2024 11:09
Elisa-Visentin pushed a commit that referenced this pull request Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Maintenance related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Saved telescope positions in MC are "wrong"
2 participants