Skip to content
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

HGCal raw data handling: Unpacker, Geometry, Local Reco #45665

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

IzaakWN
Copy link
Contributor

@IzaakWN IzaakWN commented Aug 7, 2024

PR description:

This adds work from the Raw Data Handling Group of the HGCal DPG.

This PR includes:

  1. Updates to the unpacker (for the test beam last week)
    • Follow up on PR HGCal emulator and unpacker #41461.
    • We moved the emulators to more appropriate location (EventFilter/HGCalRawToDigiSimCalorimetry/HGCalSimAlgos), they will be subject of a future PR.
  2. Updates to the logical mapping & (dense) indexing in Geometry/HGCalMapping.
  3. The introduction of electronics configuration & calibrations, read from JSONs in CondFormats.
    • No need to create special tag for these records yet as this is work in progress.
  4. New Alpaka SoA formats for DIGI and RecHits in DataFormats/HGCalDigi/interface/HGCalDigiSoA.h.
  5. Alpaka calibration kernels for the DIGI → RecHit step.
  6. Adding TH2Poly for DQM. Removed from this PR (see below).

We kindly request this to be merged into CMSSW_14_1_0_pre7 in time for the coming September test beams where validation will continue.

PR validation:

No CMSSW workflow is impacted by this PR.

We validated this with test beam data last week. You can find latest DQM plots here: http://hgc-vm-2024.cern.ch:8070/dqm/online-dev/session

Further documentation

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 7, 2024

cms-bot internal usage

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 7, 2024

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45665/41204

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

@pfs
Copy link
Contributor

pfs commented Aug 7, 2024

assign hgcal-dpg

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 7, 2024

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45665/41205

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 7, 2024

New categories assigned: hgcal-dpg

@rovere,@cseez,@pfs,@felicepantaleo you have been requested to review this Pull request/Issue and eventually sign? Thanks

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 7, 2024

A new Pull Request was created by @IzaakWN for master.

It involves the following packages:

  • CondFormats/DataRecord (alca, db)
  • CondFormats/HGCalObjects (alca, upgrade, db)
  • DQMServices/Core (dqm)
  • DataFormats/HGCalDigi (upgrade, simulation)
  • DataFormats/HGCalRecHit (****)
  • DataFormats/Histograms (core, dqm)
  • EventFilter/HGCalRawToDigi (reconstruction, upgrade)
  • Geometry/HGCalMapping (geometry, upgrade)
  • RecoLocalCalo/HGCalRecAlgos (reconstruction, upgrade)
  • SimCalorimetry/HGCalSimAlgos (upgrade, simulation)
  • SimCalorimetry/HGCalSimProducers (upgrade, simulation)

The following packages do not have a category, yet:

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

@Dr15Jones, @antoniovagnerini, @bsunanda, @civanch, @cmsbuild, @consuegs, @cseez, @felicepantaleo, @francescobrivio, @jfernan2, @makortel, @mandrenguyen, @mdhildreth, @nothingface0, @perrotta, @pfs, @rovere, @rvenditti, @saumyaphor4252, @smuzaffar, @srimanob, @subirsarkar, @syuvivida, @tjavaid can you please review it and eventually sign? Thanks.
@JanChyczynski, @Martin-Grunewald, @PonIlya, @ReyerBand, @apsallid, @argiro, @barvic, @bsunanda, @cseez, @edjtscott, @fabiocos, @felicepantaleo, @hatakeyamak, @lgray, @martinamalberti, @missirol, @mmusich, @pfs, @rchatter, @rovere, @rsreds, @sameasy, @seemasharmafnal, @sethzenz, @thomreis, @tocheng, @vandreev11, @wang0jin, @youyingli, @yuanchao this is something you requested to watch as well.
@antoniovilela, @mandrenguyen, @rappoccio, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

@@ -138,6 +138,7 @@ struct MonitorElementData {
TH2S = 0x21,
TH2D = 0x22,
TH2I = 0x23,
TH2Poly = 0x24,
Copy link
Contributor

@makortel makortel Aug 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to incorporate the TH2Poly via the existing PR #41932 instead of part of a many-thousand-line PR.

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 8, 2024

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45665/42130

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

IzaakWN pushed a commit to CMS-HGCAL/cmssw that referenced this pull request Oct 8, 2024
Code format HGCalMappingESSourceTester.cc

Removing unused includes and delarations after PR comments

Correction: missing :: for namespace

Fix code style indentation

record descriptions

Re-introducing headers.h file needed for serialization classes

remove data and change locations to cms-data folder

moving to single record as all conds produced have the same IOV

re-introducing 1 record per payload, addressing additional code comments

moving to printf, removing unused product, and message logger

code checks

remove unused iomanip

adding digi SoAs

update collection names

moving emulators to SimCalorimetry/HGCalSimAlgos, adapting paths

moving digi soa filler to alpaka and adding extra tools

some progress with soafiller

updating

fixing but in module es producer (assignment of capture block index instead of capture block)

add dense index map for HGCalElectronicsId

add getMaxDataSize & getMaxERxSize functions

fix getDenseIndex bug; add typcode map to ModuleIndexer (to retrieve module dense index offset); use regexp for type code

add getFedAndModuleIndex; rename getDenseIndex -> getIndexForModule(Data) for elecID

add calibrations Alpaka ESProducer

rename ES_HGCal*Parameter.cc -> HGCal*ParameterHostCollection.cc for clarity

int -> uint32_t; uncomment debugging print out

clean

add HGCal configuration ESProducers + record + tester

rename ES_HGCal*Parameter.cc -> HGCal*ParameterHostCollection.cc for clarity

rename HGCal*ParameterHostCollection.cc -> HGCal*ParameterDeviceCollection.cc for clarity

rename 'HGCal.*(Device|Host)Collection -> (Device|Host)' for readability; move calib SoAs to CondFormats/HGCalObjects/; move TestHGCalRecHitESProducers to test

rename 'HGCal.*(Device|Host)Collection -> HGCal.*(Device|Host)' for readability

add HGCalRecHit

add DIGI2RECO calibration algorithms (see https://gitlab.cern.ch/hgcal-dpg/hgcal-comm/-/merge_requests/3)[C; remove detId/elecId from SoAs (should be redundant since common indexer); add HGCROCChannelDataFrame<HGCalDetId> to classes_def.xml

Move HGCalRawDataDefinitions.h.

[WIP] Refactor unpacker.

parse arrays of gain-dependent calibration parameters in eRx-blocks

add missing Noise attribute

updating usage of Rcd

fixing cfg files

Convert ECON-D payload to 32b words.

[Unpacker] rebase the unpacker on denseIdx and SoA digis

Update unpacker code.

SoA format of HGCal ECON-D header

Unpack header information to HGCalECONDInfoSoA; Add in quality checks for ECON-D

remove unused

add typecode regexp; pass typecode to modIndexer_.processNewModule

update ESProducers

update record

fix ESInputTags

prefix filename with T_EventSetup_

add HGCalConfiguration & HGCalConfigurationESProducer

Update and rename HGCalRecHitProducer.cc to HGCalRecHitProducers.cc

fill HGCal config structs for unpacker (header marks, charMode)

update calibration kernels

read HGCal FED & ECON-D configuration from JSON

Update unpacker to use HGCalConfiguration structure

make manual overrides truly optional

fix compilation issue due to initialization of const gain

Change to LogDebug

adding dense index info producer

adding hgcalmapping customise

fix misunderstanding in max data size expected from indexer

Implement TH2Poly in DQM based on 14e856f

Update HGCalConfigurationESProducer.cc

update unpacker and HGCalConfiguration

Add definitions of flags in digi SOA

update the other part to most recent commit

Add flags for passthrough ECON-Ds

Debug: loop through every denseIdx

Fix the number of channel in eRx;Add default flag for digi, representing digi not read in raw data

fully read config parameters from JSONs for config SoA; add sanity checks; add HGCalMappingModuleIndexer::getNFED; rename getFedAndModuleIndex -> getIndexForFedAndModule; clean code; ...

rename macro

silence debugging message in ESProducers

silence cout

add HGCalMappingModuleIndexer::getMaxModuleSize(fedid)

typo; SystemTestEventFilters/data -> Configuration/data

bugfix in the cm Sum

fixing dense index issue

Update comment in HGCalMappingParameterSoA.h

Some additional fixes (#132)

- remove_if was missing a call to std::vector::erase to effectively remove unused entries
- typecodemap was missing a reset with the final indices
- usage of command line options was missing in module map tester
- do not use moduleLUT in DenseIndexInfoESProducer!! it has the full dimensions before trimming to the actual number of ECON-Ds in a FED

Add parsing function for econd flag; renaming HGCalECONDInfo to HGCalECONDPacketInfo

Add irot to module mapper (#134)

* - remove_if was missing a call to std::vector::erase to effectively remove unused entries
- typecodemap was missing a reset with the final indices
- usage of command line options was missing in module map tester

* do not use moduleLUT in DenseIndexInfoESProducer!!

* Adding optional irot to module mapper file
- If existing it will be put in the module info SoA
- othwerwise a default irot=0 (baseline is used)

Suppress "unused variable" warning: remove CellIndexer in HGCalDigiSoaFiller.cc

apply code formats (#135)

Apply suggestions from code review

Apply easy suggestions by Matti. (Looking into other comment & suggestions.)

Co-authored-by: Matti Kortelainen <matti.kortelainen@cern.ch>

'const json' to fix compilation issue for Matti's suggestion

gain index should be taken as is, not subtracted by 1

make use of valid at rechit level / set rechit valids

fill rec hitflags

update kernel after closure

Implement suggestions for CMSSW_14_1_X PR cms-sw#45665 (#137)

* Implement TH2Poly in DQM Services for HGCal DQM
* Apply changes from code quality checks
* Add a patch based on makortel's comments
* enum -> namespace + constexpr uint{8,16,32}_t
* Matti suggestion: remove '_t' postfix from HGCalFEDReadoutSequence_t, HGCalROCConfig_t, HGCalECONDConfig_t, HGCalFedConfig_t structs
* Matti suggestions & code checks: remove redundant comments; silence debugging cout or use MessageLogger; move includes; remove trailing '_' from function arguments
* Matti suggestion: reuse typecodeMap_.find() to avoid searching twice (TODO: throw exception if not found?)
* Matti suggestion: use SET_PORTABLEHOSTCOLLECTION_READ_RULES; add ClassVersion+checksum
* Matti suggestion: iRecord.getRecord<...>. -> iRecord.
* Matti suggestion: iRecord.getRecord<...>. -> iRecord.; rm redundant includes & couts; pass reference to fill_SoA_column to avoid copy
* Matti suggestions: use SET_PORTABLEHOSTCOLLECTION_READ_RULES for HGCalRecHitHost; Remove deprecated HGCalFlaggedECONDInfo* & comments;
* Matti suggestion: throw exception if type code not found; replace for-loop with std::accumulate
* Matti suggestion: remove TestObjects
* Matti suggestions: remove config as member data and produce unique_ptr instead of shared_ptr; NULL → nullptr; initialize optional override; memcp -> std::copy
* Matti suggestions: use file in path
* removing soafiller and updating buildfile (#139)
* address comments on HGCalRecHitCalibrationAlgorithms.dev.cc
* irot as uint8_t instead of char
* restore DQM without TH2Poly
* apply code formats

---------

Co-authored-by: ywkao <ywkao@hep1.phys.ntu.edu.tw>
Co-authored-by: Pedro <psilva@cern.ch>

restore release base DQM classes (#140)

move to chrono calls instead of struct inside namespace (#141)

address comments (#142)

update include (#143)

remove ECON-D tests as they'll need to be re-written in a different form (#144)

Declare HGCalRecHitCalibrationAlgorithms::calibrate as const method (#145)

* remove ECON-D tests as they'll need to be re-written in a different form

* enable const

patching unpacker for padding of capture blocks within FED

move check of padding to start of capture block

Fix the throw, so the processing won't stop at the first event with error

Update HGCalRecHitCalibrationAlgorithms.dev.cc

CM sum -> 0.5*CM sum

Change the throws to specific returns

Optimize the producer

Debugging the padding of eRx subpackets

roll back (#149)

apply code checks and remove dqm
@IzaakWN IzaakWN force-pushed the dev/hackathon_base_CMSSW_14_1_X branch from 6460d96 to 746003a Compare October 8, 2024 17:31
@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 8, 2024

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 8, 2024

Pull request #45665 was updated. @Moanwar, @atpathak, @cmsbuild, @consuegs, @cseez, @felicepantaleo, @francescobrivio, @fwyzard, @jfernan2, @makortel, @mandrenguyen, @perrotta, @pfs, @rovere, @srimanob, @subirsarkar can you please check and sign again.

* Add in common mode in ECOND packet information

* Enable FED-level Parallelization

* Remove change of location when something wrong in S-Link traier
@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45665/42240

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants