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

Simplify ODD Strip Module Handling, main branch (2024.08.20.) #686

Merged
merged 1 commit into from
Aug 20, 2024

Conversation

krasznaa
Copy link
Member

Removed the traccc::pixel_data::variance_y variable. As discussed in #627 (comment), there was no reason to keep this variable in memory. The values coming from the digitization files were the same as what we get with the formula implemented in the measurement creation code.

At the same time made the code for detecting 1D modules a lot more robust. Now the decision is made based on the number of "bins" in a module's segmentation.

I tested on our multi-muon ODD simulations that these changes don't affect the output of traccc_seq_example. (While also testing that for instance setting all modules to be 2D would have a visible effect.)

I wanted to put this in before making the same simplification in #627 as well. 🤔

@krasznaa krasznaa added the improvement Improve an existing feature label Aug 20, 2024
Copy link
Contributor

@beomki-yeo beomki-yeo left a comment

Choose a reason for hiding this comment

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

This also looks good to me

As it turned out, there was no reason to keep this variable in
memory. The values coming from the digitization files were the same
as what we get with the formula implemented in the measurement
creation code.

At the same time made the code for detecting 1D modules a lot more
robust. Now the decision is made based on the number of "bins" in
a module's segmentation.
@krasznaa krasznaa force-pushed the SimplifyStrips-main-20240819 branch from 6f3ee1a to 513eb04 Compare August 20, 2024 12:41
@krasznaa krasznaa enabled auto-merge August 20, 2024 12:49
@krasznaa krasznaa merged commit cb47706 into acts-project:main Aug 20, 2024
18 of 24 checks passed
@krasznaa krasznaa deleted the SimplifyStrips-main-20240819 branch August 20, 2024 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improve an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants