TkDQM: fix reset counter logic in SiPixelDigiSource #32875
Conversation
sroychow
changed the title
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32875/21105
|
|
A new Pull Request was created by @sroychow (Suvankar Roy Chowdhury) for master. It involves the following packages: DQM/SiPixelMonitorDigi @andrius-k, @kmaeshima, @ErnestaP, @ahmad3213, @cmsbuild, @jfernan2, @rvenditti can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
| unsigned int currentLS = lumi.id().luminosityBlock(); | ||
| bool resetCounters = (currentLS % 10 == 0) ? true : false; | ||
| return std::make_shared<bool>(resetCounters); | ||
| auto lsc = std::make_shared<SiPixelDigiCounter>(); |
There was a problem hiding this comment.
I suspect this is not what you want. Now a new SiPixelDigiCounter is created for every lumi, and in principle the "reset" logic below is unnecessary (except it sets the booleans to true while the constructor sets them to false).
What I tried to suggest in #32805 (comment) would be to have a member variable e.g. std::shared_ptr<SiPixelDigiCounter> latestCounters_, and then here
if (not modOn or resetCounters) {
latestCounters_ = std::make_shared<SiPixelDigiCounter>();
}and have all the resetting logic in the constructor of SiPixelDigiCounter.
| return std::make_shared<bool>(resetCounters); | ||
| auto lsc = std::make_shared<SiPixelDigiCounter>(); | ||
|
|
||
| if (modOn && resetCounters && averageDigiOccupancy) { |
There was a problem hiding this comment.
When can the averageDigiOccupancy part evaluate to false?
(I didn't see how it could be, but if I missed something, the SiPixelDigiCounter would likely have to split into two classes, one for counters being reset when averageDigiOccupancy == true and another being reset independently of averageDigiOccupancy)
|
please test |
|
please abort |
|
@sroychow what are your plans for this PR? |
|
ping @sroychow |
|
@sroychow can we close this if this is not active? |
|
-1 |
PR description:
A follow up to #32805 fixing the logic of counter reset after 10 lumis. @makortel I have tried to implement what you had suggested in the earlier PR. Please have a look.
PR validation:
Test with : runTheMatrix.py -t 4 -l 4.53
if this PR is a backport please specify the original PR and why you need to backport that PR:
NA