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
Fixing issue of PS cluster matched to >1 EE cluster in DBG_X IBs #36655
Conversation
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36655/27680
Code check has found code style and quality issues which could be resolved by applying following patch(s)
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36655/27681
|
A new Pull Request was created by @swagata87 (Swagata Mukherjee) for master. It involves the following packages:
@jpata, @cmsbuild, @clacaputo, @slava77 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
please test for CMSSW_12_3_DBG_X |
please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-867ebb/21579/summary.html Comparison SummarySummary:
|
-1 Failed Tests: RelVals RelValsThe relvals timed out after 4 hours.
|
As far as I can tell the crash is related to this PR. @swagata87 please check. |
To clarify, the debug build segfaulted in 25202.0. Is this supposed to happen? |
Hi Joosep, from a quick look, it seems to me that this crash is due to a different problem. Will look into it more.. |
please test for CMSSW_12_3_DBG_X |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-867ebb/21721/summary.html Comparison SummarySummary:
|
by the way , Step1 for few workflows [a] are taking too long for DBG IBs [a]. Although I have increased the timeout for DBG PR tests to be 9 hours butwith this rate they might fail again. Can someone check what takes stoo long for DBG relvals?
|
-1 Failed Tests: RelVals RelValsThe relvals timed out after 4 hours. |
@smuzaffar I've checked running locally
and inspecting the |
@jpata, @clacaputo, @slava77 can you please review and sign it? It should fix DBG relvals |
Due to timeout issue, I've run the test locally in
both without and with the PR applied. With the PR applied, there aren't any more WF failing with the message About [1]
|
test parameters:
|
@cmsbuild please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-867ebb/21758/summary.html Comparison SummarySummary:
|
Hi @smuzaffar , despite the test successfully passing #36655 (comment) I can still see, in the summary of the checks, one that failed. The one that failed it's related to the test performed on Is there a way to reset it? |
The failed check is part of an optional test ( |
+reconstruction
|
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. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
PR description:
Currently several workflows of debug IBs fail with the error message
Found a PS cluster matched to more than one EE cluster!
. This was reported in #34097 and #35524 . This PR is aimed at fixing this.It looks like the problem was here:
cmssw/RecoEcal/EgammaClusterAlgos/src/PFECALSuperClusterAlgo.cc
Lines 420 to 422 in b4bb14f
In particular,
i.key()
seems to give a garbage value. To bypass the issue,std::find_if
is replaced bystd::find
in a way that we do not need to access.key()
ofCaloCluster
anymore. This is similar to what was done here:cmssw/RecoParticleFlow/PFProducer/src/PFEGammaAlgo.cc
Lines 1869 to 1870 in cc5a950
This small change seems to solve the problem.
PR validation:
Tested from
CMSSW_12_3_DBG_X_2022-01-06-2300
using workflows117.0
and11630.0
.This PR is not a backport.
Backport not needed.
PS: After the first round of tests, another crash was found in DBG build (only in WF
25202.0
):I took the opportunity to fix that as well.