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 photon ID inputs when noise is on #122

Merged

Conversation

giovannimarchiori
Copy link
Contributor

When calculating RMS width of showers in layers of ECAL when noise is on the RMS can be negative due to negative-energy cells. In that case the code was crashing. This PR will set the width to zero for those cases

// Negative values can happen when noise is on and not filtered
// Negative values very close to zero can happen due to numerical precision
if (w_theta2 < 0.) {
warning() << "w_theta2 in theta width calculation is negative: " << w_theta2 << " , will set theta width to zero (this might happen when noise simulation is on)" << endmsg;
Copy link
Contributor

Choose a reason for hiding this comment

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

If this happens frequently when noise is ON, shall we have this print only for verbose?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are cases when the computation is negative when noise is OFF just due to numerical precision, and also for the noise case I think for the user it might be useful to get a least few warnings to make them aware. I have committed a version of the code in which the maximum number of warning statements over the whole execution is configurable and is by default set to 10

@BrieucF
Copy link
Contributor

BrieucF commented Oct 24, 2024

LGTM! @kjvbrt ?

@kjvbrt kjvbrt merged commit b3ebe39 into HEP-FCC:main Oct 25, 2024
4 checks passed
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.

3 participants