HGC Realistic DIGIS - part I #38538

pfs · 2022-06-28T11:11:49Z

PR description:

Context: The PR is part of a series to follow which aim to restructuring the DIGI step for HGCAL. Instead of patching the current code it is forseeen that the restructured DIGI step co-exists temporarily in CMSSW while being validated by the DPG.
Such staggered discussion was discussed after the presentation in the SIM meting of 3/12/2022 (https://indico.cern.ch/event/1101457/#67-hgcal-simulation-status) and more recently in the DPG (https://indico.cern.ch/event/1166838/#3-challenges-and-progress-towa)

Content: This PR introduces a realistic HGCROC channel data format according to the specs. It will be used as a baseline for the restructuring of the DIGI step for HGCAL, but it's not yet used elsewhere in CMSSW and as such it can't be tested with any workflow. Instead a unit test is provided with this DataFormat to make sure it performs as expected. The PR includes the move of the PHGCSimAccumulator.h class to this new directory as it will be re-used in the restructured DIGI step.

PR validation:

Using the unit test/HGCROCSampleTest.cpp provided with the PR

…ne in the future) Introduce realistic ROC channel dataframe format and associated tester Moved SimHit accumulator utility to new directory

cmsbuild · 2022-06-28T11:18:33Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38538/30771

This PR adds an extra 28KB 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-38538/30771/code-format.patch
e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38538/30771/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

cmsbuild · 2022-06-28T11:27:17Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38538/30772

This PR adds an extra 28KB to repository

cmsbuild · 2022-06-28T11:27:44Z

A new Pull Request was created by @pfs (Pedro Silva) for master.

It involves the following packages:

DataFormats/HGCDigi (upgrade, simulation)
DataFormats/HGCalDigi (****)
SimCalorimetry/HGCalSimProducers (upgrade, simulation)

The following packages do not have a category, yet:

DataFormats/HGCalDigi
Please create a PR for https://github.com/cms-sw/cms-bot/blob/master/categories_map.py to assign category

@cmsbuild, @AdrianoDee, @srimanob, @civanch, @mdhildreth can you please review it and eventually sign? Thanks.
@vandreev11, @sethzenz, @bsunanda, @rovere, @lgray, @cseez, @apsallid, @hatakeyamak, @trtomei, @beaucero 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

This stems from cms-sw/cmssw#38538 where the context is given

pfs · 2022-06-28T11:38:59Z

please test

cmsbuild · 2022-06-28T15:40:04Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-86cd07/25867/summary.html
COMMIT: 0165e85
CMSSW: CMSSW_12_5_X_2022-06-28-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/38538/25867/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: 10 differences found in the comparisons
DQMHistoTests: Total files compared: 50
DQMHistoTests: Total histograms compared: 3654586
DQMHistoTests: Total failures: 19
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 3654544
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.004 KiB( 49 files compared)
DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
Checked 208 log files, 45 edm output root files, 50 DQM output files
TriggerResults: no differences found

civanch · 2022-06-28T18:08:55Z

@pfs , should tests be *.cpp files or *.cc ?

pfs · 2022-06-28T19:43:19Z

@pfs , should tests be *.cpp files or *.cc ?

I don't know this one. I don't find references to .cpp in https://cms-sw.github.io/cms_coding_rules.html maybe that's a hint i should change to .cc?
In DataFormats/Common/test i see a mixture of both.

civanch · 2022-06-30T21:52:29Z

It is not a strong comment but better to use *.cc

pfs · 2022-07-01T07:07:59Z

Thanks, I have changed accordingly. If there are no other concerns and this, (and also #38548, and #38547) can be moved forward it would be much appreciated.

cmsbuild · 2022-07-01T07:11:23Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38538/30825

This PR adds an extra 16KB to repository
Found files with invalid states:
- DataFormats/HGCalDigi/test/HGCROCSampleTest.cpp:
  - Added: e7a156e
  - Modified: 0165e85
  - Deleted: fdd4434

cmsbuild · 2022-07-01T07:11:43Z

Pull request #38538 was updated. @cmsbuild, @AdrianoDee, @srimanob, @civanch, @mdhildreth can you please check and sign again.

civanch · 2022-07-01T08:10:36Z

please test

cmsbuild · 2022-07-01T12:06:12Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-86cd07/25932/summary.html
COMMIT: fdd4434
CMSSW: CMSSW_12_5_X_2022-06-30-2300/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/38538/25932/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: 50
DQMHistoTests: Total histograms compared: 3654771
DQMHistoTests: Total failures: 8
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3654741
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 49 files compared)
Checked 208 log files, 45 edm output root files, 50 DQM output files
TriggerResults: no differences found

civanch · 2022-07-01T15:16:08Z

+1

srimanob · 2022-07-01T15:35:21Z

SimCalorimetry/HGCalSimProducers/interface/HGCDigitizer.h

@@ -11,7 +11,7 @@

 #include "SimCalorimetry/HGCalSimProducers/interface/HGCDigitizerBase.h"
 #include "DataFormats/HGCDigi/interface/HGCDigiCollections.h"
-#include "DataFormats/HGCDigi/interface/PHGCSimAccumulator.h"
+#include "DataFormats/HGCalDigi/interface/PHGCSimAccumulator.h"


Could you please clarify what is the different of HGCDigi and HGCalDigi subpackages?

I would like to dephase HGCDigi in the future and use a name which is more inline with the one used the SimProducers/HGCal{SimAlgos,SimProducers} i.e. starting with HGCal not HGC

The PHGCSimAccumulator.h class will still be used so I moved it's location, but the new DataFormat structure (HGCROCChannelDataFrame.h) is totally different

srimanob · 2022-07-03T02:49:11Z

+Upgrade

cmsbuild · 2022-07-04T09:57:27Z

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 · 2022-07-04T12:48:49Z

As suggested by the static analyzer, in SimCalorimetry/HGCalSimProducers/plugins/HGCDigitizer.cc lines from 210 to 212 are useless: would you like to profit of this PR to clean them up?

pfs · 2022-07-04T13:34:10Z

I'll do it in the upcoming PR end of this week which will impact SimCalorimetry/HGCalSim(Producers,Algos) Thanks

…

On Mon, 4 Jul 2022, 14:49 Andrea Perrotta, ***@***.***> wrote: As suggested by the static analyzer, in SimCalorimetry/HGCalSimProducers/plugins/HGCDigitizer.cc lines from 210 to 212 are useless: would you like to profit of this PR to clean them up? — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: ***@***.***>

perrotta · 2022-07-04T14:04:43Z

+1

pfs added 2 commits May 31, 2022 22:19

Introduce new directory for DIGI dataformats (will supersed current o…

e7a156e

…ne in the future) Introduce realistic ROC channel dataframe format and associated tester Moved SimHit accumulator utility to new directory

update new location of the header file

ebb31ed

cmsbuild added this to the CMSSW_12_5_X milestone Jun 28, 2022

cmsbuild added code-checks-pending new-package-pending orp-pending pending-signatures simulation-pending tests-pending upgrade-pending labels Jun 28, 2022

cmsbuild added code-checks-rejected and removed code-checks-pending labels Jun 28, 2022

apply code formats

0165e85

cmsbuild added code-checks-pending and removed code-checks-rejected labels Jun 28, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Jun 28, 2022

pfs added a commit to pfs/cms-bot that referenced this pull request Jun 28, 2022

Update categories_map.py with new HGCalDigis

a549501

This stems from cms-sw/cmssw#38538 where the context is given

cmsbuild added tests-started and removed tests-pending labels Jun 28, 2022

cmsbuild added tests-approved and removed tests-started labels Jun 28, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Jul 1, 2022

cmsbuild added tests-started and removed tests-pending labels Jul 1, 2022

cmsbuild added tests-approved and removed tests-started labels Jul 1, 2022

cmsbuild added simulation-approved and removed simulation-pending labels Jul 1, 2022

srimanob reviewed Jul 1, 2022

View reviewed changes

cmsbuild added upgrade-approved and removed upgrade-pending labels Jul 3, 2022

qliphy mentioned this pull request Jul 4, 2022

add DataFormats/HGCalDigi cms-sw/cms-bot#1781

Merged

cmsbuild added fully-signed and removed pending-signatures new-package-pending labels Jul 4, 2022

cmsbuild added orp-approved and removed orp-pending labels Jul 4, 2022

cmsbuild merged commit 65be446 into cms-sw:master Jul 4, 2022

forthommel mentioned this pull request Apr 28, 2023

HGCal emulator and unpacker #41461

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HGC Realistic DIGIS - part I #38538

HGC Realistic DIGIS - part I #38538

pfs commented Jun 28, 2022

cmsbuild commented Jun 28, 2022

cmsbuild commented Jun 28, 2022

cmsbuild commented Jun 28, 2022

pfs commented Jun 28, 2022

cmsbuild commented Jun 28, 2022

civanch commented Jun 28, 2022

pfs commented Jun 28, 2022

civanch commented Jun 30, 2022

pfs commented Jul 1, 2022

cmsbuild commented Jul 1, 2022

cmsbuild commented Jul 1, 2022

civanch commented Jul 1, 2022

cmsbuild commented Jul 1, 2022

civanch commented Jul 1, 2022

srimanob Jul 1, 2022

pfs Jul 1, 2022

srimanob commented Jul 3, 2022

cmsbuild commented Jul 4, 2022

perrotta commented Jul 4, 2022

pfs commented Jul 4, 2022 via email

perrotta commented Jul 4, 2022

HGC Realistic DIGIS - part I #38538

HGC Realistic DIGIS - part I #38538

Conversation

pfs commented Jun 28, 2022

PR description:

PR validation:

cmsbuild commented Jun 28, 2022

cmsbuild commented Jun 28, 2022

cmsbuild commented Jun 28, 2022

pfs commented Jun 28, 2022

cmsbuild commented Jun 28, 2022

Comparison Summary

civanch commented Jun 28, 2022

pfs commented Jun 28, 2022

civanch commented Jun 30, 2022

pfs commented Jul 1, 2022

cmsbuild commented Jul 1, 2022

cmsbuild commented Jul 1, 2022

civanch commented Jul 1, 2022

cmsbuild commented Jul 1, 2022

Comparison Summary

civanch commented Jul 1, 2022

srimanob Jul 1, 2022

Choose a reason for hiding this comment

pfs Jul 1, 2022

Choose a reason for hiding this comment

srimanob commented Jul 3, 2022

cmsbuild commented Jul 4, 2022

perrotta commented Jul 4, 2022

pfs commented Jul 4, 2022 via email

perrotta commented Jul 4, 2022