[14_1_X] Added more ZDC functionality to DQM #46407

cfmcginn · 2024-10-16T14:14:21Z

Code reformatted, emulator updates, and histogram title change

PR description:

This is a backport to CMSSW_14_1_X of existing PR here: #45948

The PR adds to the DQM for ZDC specific histograms, via ZDCQIE10Task

The backport is updated relative to the main PR, mostly fixing build issue, retitling some histograms, and updating python such that emulated ZDC sums fill. It is also rebased to 14_1_X for the backport.

PR validation:

PR is tested in CMSSW_14_1_X_2024-10-15-2300, after running a merge-topic to incorporate the PR into a fresh area
scram b ran successfully, validating the build

Following test command produces the expected output:

cmsRun DQM/Integration/python/clients/hcal_dqm_sourceclient-live_cfg.py inputFiles=/eos/cms/store/group/dpg_hcal/comm_hcal/PFG/backup_raw/HIHLTPhysics-HIRun2023A-RAW-v1-375754.root runkey=hi_run

under the upload directory. Inspected histograms inside the DQM file appear reasonable for ZDC sums.

In addition, the following set of commands are run
scram b runtests
scram build code-checks
scram build code-format

code-checks revealed no issues, code-format suggestions were adopted. runtests had multiple failures, detailed below - however, every failure existed already with runtests in a fresh area of CMSSW_14_1_X_2024-10-15-2300 with the same packages checked out (before merge-topic), so it is unclear it is the result of the PR

>> Local Products Rules ..... started
>> Local Products Rules ..... done
Creating test log file logs/el9_amd64_gcc12/testing.log
Fail   26s ... DQM/Integration/TestDQMOnlineClient-beam_dqm_sourceclient
Pass   38s ... DQM/Integration/TestDQMOnlineClient-beamhlt_dqm_sourceclient
Fail    6s ... DQM/Integration/TestDQMOnlineClient-beampixel_dqm_sourceclient
Fail   49s ... DQM/Integration/TestDQMOnlineClient-csc_dqm_sourceclient
Fail    7s ... DQM/Integration/TestDQMOnlineClient-ctpps_dqm_sourceclient
Fail    6s ... DQM/Integration/TestDQMOnlineClient-dt4ml_dqm_sourceclient
Fail    6s ... DQM/Integration/TestDQMOnlineClient-dt_dqm_sourceclient
Fail    6s ... DQM/Integration/TestDQMOnlineClient-ecal_dqm_sourceclient
Pass   17s ... DQM/Integration/TestDQMOnlineClient-ecalgpu_dqm_sourceclient
Fail    7s ... DQM/Integration/TestDQMOnlineClient-es_dqm_sourceclient
Fail    6s ... DQM/Integration/TestDQMOnlineClient-fed_dqm_sourceclient
Fail    7s ... DQM/Integration/TestDQMOnlineClient-gem_dqm_sourceclient
Fail    6s ... DQM/Integration/TestDQMOnlineClient-hcal_dqm_sourceclient
Pass   10s ... DQM/Integration/TestDQMOnlineClient-hcalgpu_dqm_sourceclient
Fail    6s ... DQM/Integration/TestDQMOnlineClient-hcalreco_dqm_sourceclient
Fail    6s ... DQM/Integration/TestDQMOnlineClient-hlt_dqm_clientPB
Fail    6s ... DQM/Integration/TestDQMOnlineClient-hlt_dqm_sourceclient
Fail    5s ... DQM/Integration/TestDQMOnlineClient-info_dqm_sourceclient
Fail    6s ... DQM/Integration/TestDQMOnlineClient-l1tstage2_dqm_sourceclient
Fail    6s ... DQM/Integration/TestDQMOnlineClient-l1tstage2emulator_dqm_sourceclient
Fail    6s ... DQM/Integration/TestDQMOnlineClient-mutracking_dqm_sourceclient
Pass    9s ... DQM/Integration/TestDQMOnlineClient-onlinebeammonitor_dqm_sourceclient
Pass    7s ... DQM/Integration/TestDQMOnlineClient-pfgpu_dqm_sourceclient
Fail    6s ... DQM/Integration/TestDQMOnlineClient-pixel_dqm_sourceclient
Pass   10s ... DQM/Integration/TestDQMOnlineClient-pixelgpu_dqm_sourceclient
Fail    5s ... DQM/Integration/TestDQMOnlineClient-pixellumi_dqm_sourceclient
Fail    6s ... DQM/Integration/TestDQMOnlineClient-rpc_dqm_sourceclient
Fail    5s ... DQM/Integration/TestDQMOnlineClient-scal_dqm_sourceclient
Pass   33s ... DQM/Integration/TestDQMOnlineClient-sistrip_approx_dqm_sourceclient
Fail    6s ... DQM/Integration/TestDQMOnlineClient-sistrip_dqm_sourceclient
Fail    6s ... DQM/Integration/TestDQMOnlineClient-visualization
Fail    6s ... DQM/Integration/TestDQMOnlineClient-visualization_secondInstance
Pass  172s ... DQM/Integration/TestDQMOnlineClients-compilation_commissioning_run
Pass  142s ... DQM/Integration/TestDQMOnlineClients-compilation_cosmic_run
Pass  145s ... DQM/Integration/TestDQMOnlineClients-compilation_cosmic_run_stage1
Pass  155s ... DQM/Integration/TestDQMOnlineClients-compilation_hi_run
Pass  152s ... DQM/Integration/TestDQMOnlineClients-compilation_hpu_run
Pass  149s ... DQM/Integration/TestDQMOnlineClients-compilation_pp_run
Pass  131s ... DQM/Integration/TestDQMOnlineClients-compilation_pp_run_stage1
>> Test sequence completed for CMSSW CMSSW_14_1_X_2024-10-15-2300

If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:

As mentioned above, PR is a backport (with modification) of #45948, intended for CMSSW_14_1_X

One note - close inspection of the changes to the python file would be welcome - the changes were made as part of testing the full package but unclear to me all should be incorporated into the branch as is. Guidance appreciated.

@hjbossi @mandrenguyen

Code-reformated + title change

cmsbuild · 2024-10-16T14:14:45Z

A new Pull Request was created by @cfmcginn for CMSSW_14_1_X.

It involves the following packages:

DQM/HcalTasks (dqm)
DQM/Integration (dqm)

@antoniovagnerini, @cmsbuild, @nothingface0, @rvenditti, @syuvivida, @tjavaid can you please review it and eventually sign? Thanks.
@DryRun, @abdoulline, @batinkov, @bsunanda, @denizsun, @francescobrivio, @salimcerci, @threus 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

cmsbuild · 2024-10-16T14:14:46Z

cms-bot internal usage

mandrenguyen · 2024-10-16T14:23:01Z

please test

cmsbuild · 2024-10-16T17:38:11Z

+1

Size: This PR adds an extra 24KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a9b9d8/42239/summary.html
COMMIT: 2b258ab
CMSSW: CMSSW_14_1_X_2024-10-16-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/46407/42239/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially added 1 lines to the logs
Reco comparison results: 11 differences found in the comparisons
DQMHistoTests: Total files compared: 44
DQMHistoTests: Total histograms compared: 3331166
DQMHistoTests: Total failures: 6
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3331140
DQMHistoTests: Total skipped: 20
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 43 files compared)
Checked 191 log files, 161 edm output root files, 44 DQM output files
TriggerResults: no differences found

hjbossi · 2024-10-17T11:20:40Z

Just to add another tag for relevant reviewers: @denizsun, @salimcerci, @JoshPBR2, @abdoulline. This is a modification of of the original PR, which we intend to update with the changes once @JoshPBR2 is available to do so. The current version of the release is 14_1_3, built yesterday. If possible, we would love for this to be included the next release, i.e. 14_1_4.

As @cfmcginn mentions, it would be good to understand what is most appropriate to do for the source client file. The reason why we have questions is that I am not entirely sure what is used to create the plots online vs. local tests. Please let us know what you think!

mmusich

This PR is different from the master one: #45948, thus it's not a backport. You need to close that one and open another one that contains the same changes as this one in the master (CMSSW_14_2_X) cycle.

mmusich · 2024-10-17T11:51:04Z

DQM/HcalTasks/interface/ZDCQIE10Task.h

+ // std::shared_ptr<hcaldqm::Cache> globalBeginLuminosityBlock(edm::LuminosityBlock const &,
+ // edm::EventSetup const &) const override;
+ //void globalEndLuminosityBlock(edm::LuminosityBlock const &, edm::EventSetup const &) override;


No, removed in most recent commit

mmusich · 2024-10-17T11:52:21Z

DQM/HcalTasks/interface/ZDCQIE10Task.h

+ edm::EDGetToken sumToken_;
+ edm::ESGetToken<HcalTopology, HcalRecNumberingRecord> htopoToken_;
+ edm::ESGetToken<HcalLongRecoParams, HcalLongRecoParamsRcd> paramsToken_;
+ //edm::EDGetTokenT<l1t::EtSumBxCollection> sumZDCToken_;


Suggested change

//edm::EDGetTokenT<l1t::EtSumBxCollection> sumZDCToken_;

needed?

No, removed in most recent commit

mmusich · 2024-10-17T11:52:35Z

DQM/HcalTasks/plugins/ZDCQIE10Task.cc

 _tagQIE10 = ps.getUntrackedParameter<edm::InputTag>("tagQIE10", edm::InputTag("hcalDigis", "ZDC"));
 _tokQIE10 = consumes<QIE10DigiCollection>(_tagQIE10);
+
+ //const edm::EDGetTokenT<l1t::EtSumBxCollection> sumZDCToken_;


Suggested change

//const edm::EDGetTokenT<l1t::EtSumBxCollection> sumZDCToken_;

mmusich · 2024-10-17T11:52:43Z

DQM/HcalTasks/plugins/ZDCQIE10Task.cc

+ //const edm::EDGetTokenT<l1t::EtSumBxCollection> sumZDCToken_;
+
+ sumTag = ps.getUntrackedParameter<edm::InputTag>("etSumTag", edm::InputTag("etSumZdcProducer", ""));
+ //sumTag = ps.getParameter<edm::InputTag>("etSumTag");


Suggested change

//sumTag = ps.getParameter<edm::InputTag>("etSumTag");

mmusich · 2024-10-17T11:52:52Z

DQM/HcalTasks/plugins/ZDCQIE10Task.cc

+ sumTag = ps.getUntrackedParameter<edm::InputTag>("etSumTag", edm::InputTag("etSumZdcProducer", ""));
+ //sumTag = ps.getParameter<edm::InputTag>("etSumTag");
+ sumToken_ = consumes<l1t::EtSumBxCollection>(sumTag);
+ //sumZDCToken_ = consumes<l1t::EtSumBxCollection>(ps.getUntrackedParameter<edm::InputTag>("sumZDCToken"));


Suggested change

//sumZDCToken_ = consumes<l1t::EtSumBxCollection>(ps.getUntrackedParameter<edm::InputTag>("sumZDCToken"));

mmusich · 2024-10-17T11:59:28Z

DQM/Integration/python/clients/hcal_dqm_sourceclient-live_cfg.py

- process.load("Configuration.StandardSequences.FrontierConditions_GlobalTag_cff")
- process.GlobalTag.globaltag = autoCond['run3_data_prompt'] 
+ process.load("Configuration.StandardSequences.FrontierConditions_GlobalTag_cff")
+ process.GlobalTag.globaltag = '141X_dataRun3_Prompt_Candidate_2024_10_08_09_42_50'


don't change the default here.

Sounds good, the above is a candidate GT, but the default version will change once the new GT (containing the changes from the candidate) is in place.

This is reverted in most recent commit

mmusich · 2024-10-17T11:59:54Z

DQM/Integration/python/clients/hcal_dqm_sourceclient-live_cfg.py

@@ -57,10 +58,12 @@
 process.dqmSaverPB.runNumber = options.runNumber
 process = customise(process)
 process.DQMStore.verbose = 0
+
+'''


don't comment the lines in between.

Done, no longer commented in most recent commit

mmusich · 2024-10-17T12:00:30Z

DQM/Integration/python/clients/hcal_dqm_sourceclient-live_cfg.py

 process.hcalDigis.InputLabel = rawTag
 process.emulTPDigisNoTDCCut = process.emulTPDigis.clone(
 parameters = cms.untracked.PSet(
 ADCThresholdHF = cms.uint32(255),
 TDCMaskHF = cms.uint64(0xFFFFFFFFFFFFFFFF)
 )
 )
-process.HcalTPGCoderULUT.LUTGenerationMode = False
+process.HcalTPGCoderULUT.LUTGenerationMode = True


what's the purpose of this change?

This is actually what we would like feedback on from @denizsun and @salimcerci, this is necessary for the code to run with the candidate GT (which does not yet include an updated L1TriggerObject) , but probably not necessary for actual data-taking. In this case, should we include this or leave it out?

mmusich · 2024-10-17T12:03:13Z

DQM/Integration/python/clients/hcal_dqm_sourceclient-live_cfg.py

+process.maxEvents = cms.untracked.PSet(
+ input = cms.untracked.int32(-1)
+ )


Suggested change

process.maxEvents = cms.untracked.PSet(

input = cms.untracked.int32(-1)

)

in live mode the number of events processed is regulated elsewhere:

cmssw/DQM/Integration/python/config/inputsource_cfi.py

Lines 111 to 125 in f97584b

source = cms.Source("DQMStreamerReader",

runNumber = cms.untracked.uint32(options.runNumber),

runInputDir = cms.untracked.string(options.runInputDir),

SelectEvents = cms.untracked.vstring('*'),

streamLabel = cms.untracked.string(streamLabel),

scanOnce = cms.untracked.bool(options.scanOnce),

datafnPosition = cms.untracked.uint32(options.datafnPosition),

minEventsPerLumi = cms.untracked.int32(1),

delayMillis = cms.untracked.uint32(500),

nextLumiTimeoutMillis = cms.untracked.int32(nextLumiTimeoutMillis),

skipFirstLumis = cms.untracked.bool(options.skipFirstLumis),

deleteDatFiles = cms.untracked.bool(False),

endOfRunKills = cms.untracked.bool(endOfRunKills),

inputFileTransitionsEachEvent = cms.untracked.bool(False)

)

Removed in most recent commit

hjbossi · 2024-10-17T12:14:20Z

This PR is different from the master one: #45948, thus it's not a backport. You need to close that one and open another one that contains the same changes as this one in the master (CMSSW_14_2_X) cycle.

Hi @mmusich, thanks for your comment! The plan is to update the PR to master (CMSSW_14_2_X) cycle to have the same changes as the one here, i.e. this would be a true backport. Unfortunately, we've been unable to get ahold of Josh (I believe he might be traveling to CERN), and he is needed in order to make this change. We wanted to give ample time for review of these changes, so we decided to open the PR to 14_1_X as soon as possible. Let me know what you think of this strategy.

mmusich · 2024-10-17T12:16:15Z

Let me know what you think of this strategy.

well, abandoning the unattended PR and opening a new one from scratch will get you merged faster. This one cannot be integrated until the master one is merged.

hjbossi · 2024-10-17T12:19:01Z

Let me know what you think of this strategy.

well, abandoning the unattended PR and opening a new one from scratch will get you merged faster. This one cannot be integrated until the master one is merged.

Okay I see, in that case we will see if Josh gets back to us today, and if not, we will close the master PR and open a new one.

antoniovagnerini · 2024-10-22T09:03:27Z

Let me know what you think of this strategy.

well, abandoning the unattended PR and opening a new one from scratch will get you merged faster. This one cannot be integrated until the master one is merged.

Okay I see, in that case we will see if Josh gets back to us today, and if not, we will close the master PR and open a new one.

@hjbossi any updates on the comments by Marco? At this stage, I would also recommend closing the master #45948 and making a new one that matches the backport.

cfmcginn · 2024-10-22T09:15:59Z

Let me know what you think of this strategy.

well, abandoning the unattended PR and opening a new one from scratch will get you merged faster. This one cannot be integrated until the master one is merged.

Okay I see, in that case we will see if Josh gets back to us today, and if not, we will close the master PR and open a new one.

@hjbossi any updates on the comments by Marco? At this stage, I would also recommend closing the master #45948 and making a new one that matches the backport.

I intend to open the new PR to main today; I don't know that I have power to close the old, but that should follow

hjbossi · 2024-10-22T09:26:32Z

Looking at the master PR #45948, I don't think I have permissions to close it, which means that must be done by @JoshPBR2. Although maybe someone with less restricted permissions can do so?

cfmcginn · 2024-10-22T13:48:58Z

PR to main can be found here:

#46478

cmsbuild · 2024-10-22T16:18:30Z

Pull request #46407 was updated. @antoniovagnerini, @cmsbuild, @nothingface0, @rvenditti, @syuvivida, @tjavaid can you please check and sign again.

Rebased version of DQM PR for backport

2b258ab

Code-reformated + title change

cmsbuild added this to the CMSSW_14_1_X milestone Oct 16, 2024

cmsbuild added dqm-pending pending-signatures tests-pending orp-pending labels Oct 16, 2024

cfmcginn changed the title ~~Rebased version of DQM PR for backport~~ [14_1_X] Added more ZDC functionality to DQM Oct 16, 2024

cmsbuild added tests-started and removed tests-pending labels Oct 16, 2024

cmsbuild added tests-approved and removed tests-started labels Oct 16, 2024

mmusich reviewed Oct 17, 2024

View reviewed changes

cfmcginn mentioned this pull request Oct 22, 2024

Added more ZDC functionality to DQM #46478

Open

Update per PR feedback, largely comment removal + GT fixes

a40e87b

cmsbuild added tests-pending and removed tests-approved labels Oct 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[14_1_X] Added more ZDC functionality to DQM #46407

[14_1_X] Added more ZDC functionality to DQM #46407

cfmcginn commented Oct 16, 2024

cmsbuild commented Oct 16, 2024

cmsbuild commented Oct 16, 2024 •

edited

Loading

mandrenguyen commented Oct 16, 2024

cmsbuild commented Oct 16, 2024

hjbossi commented Oct 17, 2024

mmusich left a comment

mmusich Oct 17, 2024

cfmcginn Oct 22, 2024

mmusich Oct 17, 2024

cfmcginn Oct 22, 2024

mmusich Oct 17, 2024

cfmcginn Oct 22, 2024

mmusich Oct 17, 2024

cfmcginn Oct 22, 2024

mmusich Oct 17, 2024

cfmcginn Oct 22, 2024

mmusich Oct 17, 2024

hjbossi Oct 17, 2024

cfmcginn Oct 22, 2024

mmusich Oct 17, 2024

cfmcginn Oct 22, 2024

mmusich Oct 17, 2024

hjbossi Oct 17, 2024

mmusich Oct 17, 2024

cfmcginn Oct 22, 2024

hjbossi commented Oct 17, 2024

mmusich commented Oct 17, 2024

hjbossi commented Oct 17, 2024

antoniovagnerini commented Oct 22, 2024

cfmcginn commented Oct 22, 2024

hjbossi commented Oct 22, 2024

cfmcginn commented Oct 22, 2024

cmsbuild commented Oct 22, 2024

	process.maxEvents = cms.untracked.PSet(
	input = cms.untracked.int32(-1)
	)

	source = cms.Source("DQMStreamerReader",
	runNumber = cms.untracked.uint32(options.runNumber),
	runInputDir = cms.untracked.string(options.runInputDir),
	SelectEvents = cms.untracked.vstring('*'),
	streamLabel = cms.untracked.string(streamLabel),
	scanOnce = cms.untracked.bool(options.scanOnce),
	datafnPosition = cms.untracked.uint32(options.datafnPosition),
	minEventsPerLumi = cms.untracked.int32(1),
	delayMillis = cms.untracked.uint32(500),
	nextLumiTimeoutMillis = cms.untracked.int32(nextLumiTimeoutMillis),
	skipFirstLumis = cms.untracked.bool(options.skipFirstLumis),
	deleteDatFiles = cms.untracked.bool(False),
	endOfRunKills = cms.untracked.bool(endOfRunKills),
	inputFileTransitionsEachEvent = cms.untracked.bool(False)
	)

[14_1_X] Added more ZDC functionality to DQM #46407

Are you sure you want to change the base?

[14_1_X] Added more ZDC functionality to DQM #46407

Conversation

cfmcginn commented Oct 16, 2024

PR description:

PR validation:

If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:

cmsbuild commented Oct 16, 2024

cmsbuild commented Oct 16, 2024 • edited Loading

mandrenguyen commented Oct 16, 2024

cmsbuild commented Oct 16, 2024

Comparison Summary

hjbossi commented Oct 17, 2024

mmusich left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hjbossi commented Oct 17, 2024

mmusich commented Oct 17, 2024

hjbossi commented Oct 17, 2024

antoniovagnerini commented Oct 22, 2024

cfmcginn commented Oct 22, 2024

hjbossi commented Oct 22, 2024

cfmcginn commented Oct 22, 2024

cmsbuild commented Oct 22, 2024

cmsbuild commented Oct 16, 2024 •

edited

Loading