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

fix noise filtering bug in calo digitisation, and restore defaults #113

Merged

Conversation

giovannimarchiori
Copy link
Contributor

In the calo digitisation classes, when noise filtering is on, the cells which actually get populated with noise becomes smaller and smaller event after event due to the fact that the filtered cells are removed from the cell map (from e.g. this:

) and never re-added at the beginning of the next event.

This PR fixes the problem.

Also, a PR recently merged (#107) changed some default values of the properties of CreateCaloCells. Even though those are configurables and can be modified at python level, for backward compatibility I'd rather restore the original values as mentioned here #107 (comment)

@kjvbrt I think @BrieucF is still on vacation so could you please have a look? Since it's a bug fix this is quite urgent. When merged I can then open a follow-up PR to fix the things mentioned in #107 (comment)

Thanks, Giovanni

@kjvbrt
Copy link
Contributor

kjvbrt commented Sep 13, 2024

Hi @giovannimarchiori, merging...

@kjvbrt kjvbrt merged commit 26276b7 into HEP-FCC:main Sep 13, 2024
2 of 5 checks passed
@kjvbrt
Copy link
Contributor

kjvbrt commented Sep 13, 2024

@giovannimarchiori, why are you even keeping cells map across events? I'm not sure it saves any cpu/memory.

@giovannimarchiori
Copy link
Contributor Author

that code predates me but I think it is to avoid invoking at each event the segmentation code that returns max/min thetaID and number of theta and phi bins for each layer and creates an empty map with all possible cellIDs. Not sure how much is the gain in CPU

@giovannimarchiori giovannimarchiori deleted the gmarchio-main-20240913-fixnoisefiltering branch September 13, 2024 12:58
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

Successfully merging this pull request may close these issues.

2 participants