-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
fixed LHCInfoPer* PopCons not recoginizing ongoing fills in duringFill mode #42837
fixed LHCInfoPer* PopCons not recoginizing ongoing fills in duringFill mode #42837
Conversation
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42837/36967
|
A new Pull Request was created by @JanChyczynski (jan_chyczynski) for master. It involves the following packages:
@perrotta, @cmsbuild, @consuegs, @saumyaphor4252, @francescobrivio can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
please test |
-1 Failed Tests: UnitTests Unit TestsI found 1 errors in the following unit tests: ---> test CondToolsLHCInfoNewPopConTest had ERRORS Comparison SummaryThere are some workflows for which there are errors in the baseline: Summary:
|
@JanChyczynski the error in the unit test is related: please have a look |
8b8e2e7
to
32e430f
Compare
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42837/36991
|
Pull request #42837 was updated. @perrotta, @cmsbuild, @consuegs, @saumyaphor4252, @francescobrivio can you please check and sign again. |
32e430f
to
9f25c47
Compare
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42837/36993
|
Pull request #42837 was updated. @perrotta, @cmsbuild, @consuegs, @saumyaphor4252, @francescobrivio can you please check and sign again. |
please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e25d98/34892/summary.html Comparison SummarySummary:
|
+db |
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @sextonkennedy, @rappoccio, @antoniovilela (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
PR description:
Fixes bugs in
LHCInfoPerFill
andLHCInfoPerLS
PopCons induringFill
mode causing it not to recognize a fill as ongoing. It caused the PopCons not process any fills and not write any data in this mode.DuringFill mode is supposed to only process ongoing fills and write only one - the newest available payload for each execution during stable beam. It interprets a fill as ongoing if its end_time (taken from OMS) is null which means it didn't end. It is not checking the value for being
null
directly but checking if theend_time
converted tocond::Time_t
is equal to 0. It was not working correctly because conversion fromend_time
value taken from OMS asboost::posix_time::ptime
tocond::Time_t
convertsnull
to2035-10-29 06:32:22
.PR validation:
It was validated by running the PopCons via
LHCInfoPerFillPopConAnalyzer.py
andLHCInfoPerLSPopConAnalyzer.py
withmode=duringFill
during and outside of stable beam of an ongoing fill. It was checked if the payload was written given the right circumstances (ongoing stable beam) and if the written data is correct.Backporting
I think it should be backported to 13_2_X, but it depends on what release the Per* PopCons are going to be deployed.