Remove a dead assignment of accChargeForToA in the HGCDigitizer #39622

perrotta · 2022-10-05T09:02:27Z

PR description:

Spotted by the static analyzer

PR validation:

It builds succesfully
No changes at all expected in the outputs

perrotta · 2022-10-05T09:04:51Z

assign hgcal-dpg

cmsbuild · 2022-10-05T09:10:03Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39622/32417

This PR adds an extra 16KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File SimCalorimetry/HGCalSimProducers/plugins/HGCDigitizer.cc modified in PR(s): HGCAL: Change ToA to local time #39447

cmsbuild · 2022-10-05T09:10:29Z

New categories assigned: hgcal-dpg

@felicepantaleo,@rovere,@pfs,@cseez you have been requested to review this Pull request/Issue and eventually sign? Thanks

cmsbuild · 2022-10-05T09:10:31Z

A new Pull Request was created by @perrotta (Andrea Perrotta) for master.

It involves the following packages:

SimCalorimetry/HGCalSimProducers (upgrade, simulation)

@cseez, @civanch, @felicepantaleo, @rovere, @cmsbuild, @AdrianoDee, @srimanob, @mdhildreth, @pfs can you please review it and eventually sign? Thanks.
@youyingli, @vandreev11, @sethzenz, @bsunanda, @rovere, @lgray, @cseez, @apsallid, @pfs, @hatakeyamak, @trtomei, @beaucero this is something you requested to watch as well.
@perrotta, @dpiparo, @rappoccio you are the release manager for this.

cms-bot commands are listed here

perrotta · 2022-10-05T09:11:10Z

please test

rovere · 2022-10-05T09:14:46Z

SimCalorimetry/HGCalSimProducers/plugins/HGCDigitizer.cc

        const auto aboveThrPos = std::upper_bound(
            hitRec.begin(), hitRec.end(), std::make_pair(cell_threshold, 0.f), [](const auto& i, const auto& j) {
              return i.first < j.first;
            });

        if (aboveThrPos == hitRec.end()) {
-          accChargeForToA = hitRec.back().first;
-          fireTDC = 0.f;
+          //          fireTDC = 0.f;


Would it make the code more readable if the 2 conditions are combined?

Would it make the code more readable if the 2 conditions are combined?

Why not....
It is up to you, anyhow. I did not want to modify too much the structure of the code, but if you want I can implement it here as well.

@rovere done

cmsbuild · 2022-10-05T11:32:16Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39622/32421

This PR adds an extra 16KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File SimCalorimetry/HGCalSimProducers/plugins/HGCDigitizer.cc modified in PR(s): HGCAL: Change ToA to local time #39447

cmsbuild · 2022-10-05T11:32:37Z

Pull request #39622 was updated. @cseez, @civanch, @felicepantaleo, @rovere, @cmsbuild, @AdrianoDee, @srimanob, @mdhildreth, @pfs can you please check and sign again.

perrotta · 2022-10-05T11:38:03Z

please test

perrotta · 2023-01-20T12:01:13Z

No need to keep this PR in standby forever only because it points out a bug in the code: an issue for it has been opened in #40576 for that
Let rerun the tests here, and merge this technical PR if they still succed after 5 months

perrotta · 2023-01-20T12:01:20Z

please test

perrotta · 2023-01-20T12:01:46Z

unhold

cmsbuild · 2023-01-20T12:11:07Z

This pull request is fully signed and it will be integrated in one of the next master IBs after it passes the integration tests. This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @rappoccio (and backports should be raised in the release meeting by the corresponding L2)

cmsbuild · 2023-01-20T16:20:43Z

-1

Failed Tests: RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c3e35e/30093/summary.html
COMMIT: fc830b3
CMSSW: CMSSW_13_0_X_2023-01-20-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/39622/30093/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals-INPUT

The relvals timed out after 4 hours.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 49
DQMHistoTests: Total histograms compared: 3555479
DQMHistoTests: Total failures: 3
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3555454
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
Checked 211 log files, 162 edm output root files, 49 DQM output files
TriggerResults: no differences found

perrotta · 2023-01-21T06:38:16Z

please test

cmsbuild · 2023-01-21T09:15:06Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c3e35e/30108/summary.html
COMMIT: fc830b3
CMSSW: CMSSW_13_0_X_2023-01-20-2300/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/39622/30108/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 5 differences found in the comparisons
DQMHistoTests: Total files compared: 49
DQMHistoTests: Total histograms compared: 3555479
DQMHistoTests: Total failures: 6
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3555451
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
Checked 211 log files, 162 edm output root files, 49 DQM output files
TriggerResults: no differences found

perrotta · 2023-01-21T11:43:15Z

+1

cmsbuild added this to the CMSSW_12_6_X milestone Oct 5, 2022

cmsbuild added code-checks-pending orp-pending pending-signatures simulation-pending tests-pending upgrade-pending labels Oct 5, 2022

cmsbuild added the hgcal-dpg-pending label Oct 5, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 5, 2022

cmsbuild added tests-started and removed tests-pending labels Oct 5, 2022

rovere reviewed Oct 5, 2022

View reviewed changes

perrotta force-pushed the removeDeadAssignmentInHGCDigitizer branch from bfde294 to 1569b96 Compare October 5, 2022 11:25

cmsbuild added code-checks-pending tests-pending and removed tests-started code-checks-approved labels Oct 5, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 5, 2022

cmsbuild added tests-started and removed tests-pending labels Oct 5, 2022

cmsbuild added the pending-signatures label Oct 28, 2022

cmsbuild modified the milestones: CMSSW_12_6_X, CMSSW_13_0_X Nov 24, 2022

cmsbuild mentioned this pull request Dec 24, 2022

Phase2-hgx334B Extend some of the calling sequences of Geometry to include zside which is needed to treat correctly cassette shifts #40404

Merged

perrotta mentioned this pull request Jan 20, 2023

Possible bug in HGCDigitizer #40576

Open

cmsbuild added fully-signed tests-started and removed pending-signatures tests-rejected hold labels Jan 20, 2023

cmsbuild added tests-rejected and removed tests-started labels Jan 20, 2023

cmsbuild added tests-started and removed tests-rejected labels Jan 21, 2023

cmsbuild added tests-approved and removed tests-started labels Jan 21, 2023

cmsbuild added orp-approved and removed orp-pending labels Jan 21, 2023

cmsbuild merged commit e24ba5b into cms-sw:master Jan 21, 2023

perrotta deleted the removeDeadAssignmentInHGCDigitizer branch January 21, 2023 11:44

This was referenced Jan 21, 2023

Implement a simple vector add test with alpaka #40580

Merged

[BuildRules] Use default compiler for alpaka/serial dev.cc files cms-sw/cmsdist#8256

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove a dead assignment of accChargeForToA in the HGCDigitizer #39622

Remove a dead assignment of accChargeForToA in the HGCDigitizer #39622

perrotta commented Oct 5, 2022

perrotta commented Oct 5, 2022

cmsbuild commented Oct 5, 2022

cmsbuild commented Oct 5, 2022

cmsbuild commented Oct 5, 2022

perrotta commented Oct 5, 2022

rovere Oct 5, 2022

perrotta Oct 5, 2022

rovere Oct 5, 2022

perrotta Oct 5, 2022

cmsbuild commented Oct 5, 2022

cmsbuild commented Oct 5, 2022

perrotta commented Oct 5, 2022

perrotta commented Jan 20, 2023

perrotta commented Jan 20, 2023

perrotta commented Jan 20, 2023

cmsbuild commented Jan 20, 2023

cmsbuild commented Jan 20, 2023

perrotta commented Jan 21, 2023

cmsbuild commented Jan 21, 2023

perrotta commented Jan 21, 2023

Remove a dead assignment of accChargeForToA in the HGCDigitizer #39622

Remove a dead assignment of accChargeForToA in the HGCDigitizer #39622

Conversation

perrotta commented Oct 5, 2022

PR description:

PR validation:

perrotta commented Oct 5, 2022

cmsbuild commented Oct 5, 2022

cmsbuild commented Oct 5, 2022

cmsbuild commented Oct 5, 2022

perrotta commented Oct 5, 2022

rovere Oct 5, 2022

Choose a reason for hiding this comment

perrotta Oct 5, 2022

Choose a reason for hiding this comment

rovere Oct 5, 2022

Choose a reason for hiding this comment

perrotta Oct 5, 2022

Choose a reason for hiding this comment

cmsbuild commented Oct 5, 2022

cmsbuild commented Oct 5, 2022

perrotta commented Oct 5, 2022

perrotta commented Jan 20, 2023

perrotta commented Jan 20, 2023

perrotta commented Jan 20, 2023

cmsbuild commented Jan 20, 2023

cmsbuild commented Jan 20, 2023

RelVals-INPUT

Comparison Summary

perrotta commented Jan 21, 2023

cmsbuild commented Jan 21, 2023

Comparison Summary

perrotta commented Jan 21, 2023