Changes in HGCal geometry vs flatfile comparison #36174

yulunmiao · 2021-11-18T21:58:13Z

PR description:

Changes are made in Validation/HGCalValidation/plugins/HGCalWaferValidation.cc
to allow reading wafer information from new flat file format and compare to geometry.
and in Validation/HGCalValidation/test/python/testHGCalWaferValidation_cfg.py to allow take geometry D86 as argument and can specify certain flat file via the path to it.

More details can be found here https://indico.cern.ch/event/1095051/contributions/4610617/attachments/2344534/3997691/GeometryD86vsflatfile_yulunmiao.pdf

PR validation:

Checks using geometry D83 with old flat file format and geometry D86 with new flat file format is done, no bug is found in code

cmsbuild · 2021-11-18T21:58:36Z

A new Pull Request was created by @yulunmiao for CMSSW_12_1_X.

It involves the following packages:

Validation/HGCalValidation (dqm)

@emanueleusai, @ahmad3213, @cmsbuild, @jfernan2, @pmandrik, @pbo0, @rvenditti can you please review it and eventually sign? Thanks.
@vandreev11, @sethzenz, @bsunanda, @rovere, @lgray, @cseez, @apsallid, @pfs, @lecriste, @hatakeyamak, @ebrondol this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

qliphy · 2021-11-19T00:23:57Z

@yulunmiao Please write a meaningful PR title. Moreover, make a forward PR in master (12_2_X)

yulunmiao · 2021-11-19T13:57:43Z

Hi @qliphy , thank you for your comments, I have made the changes.

cmsbuild · 2021-11-19T14:00:26Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36174/26754

This PR adds an extra 24KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36174/26754/code-format.patch
e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36174/26754/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

perrotta · 2021-11-19T14:34:40Z

Hi @qliphy , thank you for your comments, I have made the changes.

@yulunmiao Since this PR moved to master, before we can test it you have now to apply the code-format fixes, as explained in #36174 (comment)

cmsbuild · 2021-11-19T15:50:44Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36174/26761

This PR adds an extra 24KB to repository

cmsbuild · 2021-11-19T15:51:04Z

Pull request #36174 was updated. @emanueleusai, @ahmad3213, @cmsbuild, @jfernan2, @pmandrik, @pbo0, @rvenditti can you please check and sign again.

jfernan2 · 2021-11-19T19:17:45Z

Validation/HGCalValidation/plugins/HGCalWaferValidation.cc

+        waferInfo.shapeCode = 0;
+      if (shapeStr == "a")
+        waferInfo.shapeCode = 4;
+      if (shapeStr == "am")


what prevents all these ifs to be else if?

THese should be replaced by else if too

jfernan2 · 2021-11-19T19:18:12Z

Validation/HGCalValidation/plugins/HGCalWaferValidation.cc

+          waferShapeCode = 4;
+        if (waferShapeStr == "3")
+          waferShapeCode = 5;
+        if (waferShapeStr == "4")


Same comment here: why not else if?

Not migrated yet

jfernan2 · 2021-11-19T19:19:03Z

Validation/HGCalValidation/plugins/HGCalWaferValidation.cc

+        if (waferShapeStr == "1")
+          waferShapeCode = 4;
+        if (waferShapeStr == "2")
+          waferShapeCode = 4;


Same assignment than 2 lines above?

jfernan2 · 2021-11-19T19:19:45Z

Validation/HGCalValidation/plugins/HGCalWaferValidation.cc

+        if (waferShapeStr == "3")
+          waferShapeCode = 5;
+        if (waferShapeStr == "4")
+          waferShapeCode = 5;


Same assignment than two lines above, these ifs could be merged, right?

pfs · 2021-11-30T08:27:51Z

@yulunmiao @bsunanda @imranyusuff I have a request.

In the mapping of strings to wafer types, can you use the names in the enum .
I.e. instead of lines like

if (shapeStr == "a")
        waferInfo.shapeCode = 4;

do something like

if (shapeStr == "a")
        waferInfo.shapeCode = WaferPartialType::WaferHalf;

etc. which is easier for lay people to understand and not get confused by the fact that a different numbering scheme is used outside CMSSW (cf slide 3).

yulunmiao · 2021-11-30T19:25:09Z

@pfs Thanks for you comment. I've made the changes

cmsbuild · 2021-11-30T19:32:58Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36174/27041

This PR adds an extra 28KB to repository
Found files with invalid states:
- Validation/HGCalValidation/HGCalWaferValidation.cc:
  - Added: efb70d4
  - Deleted: 7b483a1

cmsbuild · 2021-11-30T19:33:20Z

Pull request #36174 was updated. @emanueleusai, @ahmad3213, @cmsbuild, @jfernan2, @pmandrik, @pbo0, @rvenditti can you please check and sign again.

bsunanda · 2021-12-01T11:57:39Z

@cmsbuild Please test

cmsbuild · 2021-12-01T14:58:54Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-75d92e/20901/summary.html
COMMIT: 7c97a1c
CMSSW: CMSSW_12_2_X_2021-11-30-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/36174/20901/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: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 41
DQMHistoTests: Total histograms compared: 3041955
DQMHistoTests: Total failures: 6
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3041927
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 40 files compared)
Checked 175 log files, 37 edm output root files, 41 DQM output files
TriggerResults: no differences found

jfernan2 · 2021-12-01T15:54:11Z

+1

cmsbuild · 2021-12-01T15:54:36Z

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)

perrotta · 2021-12-01T16:50:49Z

+1

yulunmiao added 2 commits November 18, 2021 15:14

Allow geometry D86, argument of flat file

fd1563a

Allow reading new flat file format

51d6853

cmsbuild added this to the CMSSW_12_1_X milestone Nov 18, 2021

cmsbuild added dqm-pending orp-pending pending-signatures tests-pending labels Nov 18, 2021

yulunmiao changed the base branch from CMSSW_12_1_X to master November 19, 2021 13:55

cmsbuild modified the milestones: CMSSW_12_1_X, CMSSW_12_2_X Nov 19, 2021

cmsbuild added the code-checks-pending label Nov 19, 2021

yulunmiao changed the title ~~Yulun miao cmssw 12 1 0 pre4~~ Changes in HGCal geometry vs flatfile comparison Nov 19, 2021

cmsbuild added code-checks-rejected and removed code-checks-pending labels Nov 19, 2021

Reformatted with scram build code-format

3ad73e6

cmsbuild added code-checks-pending and removed code-checks-rejected labels Nov 19, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Nov 19, 2021

jfernan2 reviewed Nov 19, 2021

View reviewed changes

cmsbuild added the tests-approved label Nov 30, 2021

use the names in HGCalTypes.h

7c97a1c

cmsbuild added code-checks-pending tests-pending and removed tests-approved code-checks-approved labels Nov 30, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Nov 30, 2021

cmsbuild added tests-started and removed tests-pending labels Dec 1, 2021

cmsbuild added tests-approved and removed tests-started labels Dec 1, 2021

cmsbuild added dqm-approved fully-signed and removed dqm-pending pending-signatures labels Dec 1, 2021

cmsbuild added orp-approved and removed orp-pending labels Dec 1, 2021

cmsbuild merged commit bc75e59 into cms-sw:master Dec 1, 2021

This was referenced Dec 1, 2021

Update Run3 offline data GT and mcRun3 GTs with final DD4HEP #36326

Merged

Move to root 6.24 cms-sw/cmsdist#7445

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Changes in HGCal geometry vs flatfile comparison #36174

Changes in HGCal geometry vs flatfile comparison #36174

yulunmiao commented Nov 18, 2021

cmsbuild commented Nov 18, 2021

qliphy commented Nov 19, 2021

yulunmiao commented Nov 19, 2021

cmsbuild commented Nov 19, 2021

perrotta commented Nov 19, 2021

cmsbuild commented Nov 19, 2021

cmsbuild commented Nov 19, 2021

jfernan2 Nov 19, 2021

jfernan2 Nov 21, 2021

jfernan2 Nov 19, 2021 •

edited

Loading

jfernan2 Nov 21, 2021

jfernan2 Nov 19, 2021

jfernan2 Nov 21, 2021

jfernan2 Nov 19, 2021

jfernan2 Nov 21, 2021

pfs commented Nov 30, 2021 •

edited

Loading

yulunmiao commented Nov 30, 2021

cmsbuild commented Nov 30, 2021

cmsbuild commented Nov 30, 2021

bsunanda commented Dec 1, 2021

cmsbuild commented Dec 1, 2021

jfernan2 commented Dec 1, 2021

cmsbuild commented Dec 1, 2021

perrotta commented Dec 1, 2021

Changes in HGCal geometry vs flatfile comparison #36174

Changes in HGCal geometry vs flatfile comparison #36174

Conversation

yulunmiao commented Nov 18, 2021

PR description:

PR validation:

cmsbuild commented Nov 18, 2021

qliphy commented Nov 19, 2021

yulunmiao commented Nov 19, 2021

cmsbuild commented Nov 19, 2021

perrotta commented Nov 19, 2021

cmsbuild commented Nov 19, 2021

cmsbuild commented Nov 19, 2021

jfernan2 Nov 19, 2021

Choose a reason for hiding this comment

jfernan2 Nov 21, 2021

Choose a reason for hiding this comment

jfernan2 Nov 19, 2021 • edited Loading

Choose a reason for hiding this comment

jfernan2 Nov 21, 2021

Choose a reason for hiding this comment

jfernan2 Nov 19, 2021

Choose a reason for hiding this comment

jfernan2 Nov 21, 2021

Choose a reason for hiding this comment

jfernan2 Nov 19, 2021

Choose a reason for hiding this comment

jfernan2 Nov 21, 2021

Choose a reason for hiding this comment

pfs commented Nov 30, 2021 • edited Loading

yulunmiao commented Nov 30, 2021

cmsbuild commented Nov 30, 2021

cmsbuild commented Nov 30, 2021

bsunanda commented Dec 1, 2021

cmsbuild commented Dec 1, 2021

Comparison Summary

jfernan2 commented Dec 1, 2021

cmsbuild commented Dec 1, 2021

perrotta commented Dec 1, 2021

jfernan2 Nov 19, 2021 •

edited

Loading

pfs commented Nov 30, 2021 •

edited

Loading