-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
[14_1_X] Update ZdcSimpleRecAlgo_Run3.cc #46296
[14_1_X] Update ZdcSimpleRecAlgo_Run3.cc #46296
Conversation
Update ZdcSimpleRecAlgo_Run3.cc Fixed an issue where if statement checking wrong variable to guard against seg fault. Fixed issue by updating associated for loop condition to be less than digi_size. I also updated the chargedweightedtime to check for potential NAN which will now be set to -99.0
A new Pull Request was created by @matt2275 for CMSSW_14_1_X. It involves the following packages:
@cmsbuild, @jfernan2, @mandrenguyen can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
cms-bot internal usage |
please test |
+1 Size: This PR adds an extra 20KB to repository Comparison SummarySummary:
|
+1 |
This pull request is fully signed and it will be integrated in one of the next CMSSW_14_1_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_14_2_X is complete. This pull request will now be reviewed by the release team before it's merged. @antoniovilela, @rappoccio, @sextonkennedy, @mandrenguyen (and backports should be raised in the release meeting by the corresponding L2) |
backport of PR #46295 |
+1 |
PR description:
This is backport of PR #46295 for CMSSW_14_1_X
Fixed an issue where if statement checking wrong variable to guard against seg fault. Fixed issue by updating associated for loop condition to be less than
digi_size
. I also updated theChargeWeightedTime
to check for potential NAN which will now be set to -99.0This issue is not critical for 2024 HI data taking. The issue was noticed when running on outdated conditions where the signal Timeslices included 6 which caused the bad if statement to skip the calculation of the
ChargeWeightedTime
variable since the number of samples in the digi is 6. When the conditions are updated for the signal TS to be 2 and the noise TS to be 1, the if statement will be false and ChargeWeightedTime will be calculated correctly. The for loop will be unchanged sincenTS_ = digi_size = 6
. There is a small risk of seg fault if nTS_ is updated in ZdcHitReconstructorRun3 to be greater than thedigi_size
but that would not occur on global reconstructor, only local running if the user happens to update that value incorrectly.PR validation:
For testing, I ran on this script where previously, the values of the ChargeWeightedTime were NAN and now are positive floats from 0 to 125 ns.