Phase2-hgx292 Bug fix for real partial wafer simulations in versions V15 and V16 #35765
Conversation
|
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35765/26111
Code check has found code style and quality issues which could be resolved by applying following patch(s)
|
cmsbuild
added
code-checks-rejected
code-checks-pending
and removed
code-checks-pending
code-checks-rejected
labels
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35765/26112
|
|
A new Pull Request was created by @bsunanda (Sunanda Banerjee) for master. It involves the following packages:
@civanch, @Dr15Jones, @makortel, @cvuosalo, @ianna, @mdhildreth, @cmsbuild, @AdrianoDee, @srimanob can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
@cmsbuild Please test |
|
assign hgcal-dpg |
|
New categories assigned: hgcal-dpg @felicepantaleo,@rovere,@pfs,@cseez you have been requested to review this Pull request/Issue and eventually sign? Thanks |
cmsbuild
added
hgcal-dpg-pending
code-checks-pending
tests-pending
and removed
tests-started
code-checks-approved
labels
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35765/26118
|
|
+Upgrade This is a bug-fix as mentioned in the PR description. It is discussed at the HGCal meeting as mentioned above. PR test runs fine. |
|
+1 |
cmsbuild
added
fully-signed
geometry-approved
simulation-approved
and removed
simulation-pending
geometry-pending
pending-signatures
labels
|
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) |
| << ":" << xy.second; | ||
| double dx = (xx - xy.first); | ||
| double dy = (pos.y() - xy.second); | ||
| double dR = std::sqrt(dx * dx + dy * dy); |
There was a problem hiding this comment.
(For a possible fix in a forthcoming PR):
| double dR = std::sqrt(dx * dx + dy * dy); | |
| double dR2 = dx * dx + dy * dy; |
to be compared with tolR2
There was a problem hiding this comment.
We shall take care of that in future PR which we can do once this PR is merged
|
The tests show large differences in the DQM plots of wf 38634.0, which corresponds to 2026D86, i.e. V16 geometry. |
Hi @perrotta For the reco content, I'm not sure. Do you know the detail on how the comparison works? For example, I see the difference in JetMET/JetValidation report. |
|
@perrotta Please merge this PR. We are making ready the next PR which we can submit in a couple of days |
|
@bsunanda we are closing for pre5. Tonights build ended up with some unit test error, therefore we will look for a fix for them and start another build of pre5 with them. At that point, even if not necessary for 12_1_0, this PR could also get added just to speed up your integration of the next steps. To do so, I would like to have clear the following:
|
|
We had an extensive discussion in the HGCal DPG regarding this. There are clear differences in the workflows for 38634.0 and 37434.0. They mostly deal with energy deposits in the outer most and innermost partial wafers. Some of the discrepancies observed earlier were recovered with this PR. |
I think the bot was clear: |
|
@slava77 |
everything else should run, only the unmapped workflow will not get reco comparisons running. This is only about the fwlite-based comparisons that appear in validateJR. |
|
+1 |
PR description:
Bug fix for real partial wafer simulations in versions V15 and V16. This involves declaration of proper Sensitive Detector volumes and inhibition of using additional weights which was needed for virtual partial wafers used in V11..V14
PR validation:
Make use of the runTheMatrix test workflows and also overlap testing
if this PR is a backport please specify the original PR and why you need to backport that PR:
Nothing special