Miscellaneous fixes for dqm online clients

[mmusich]

PR description:

The night of 13th March 2024, run 377827 was mistakenly acquired using the DQM DAQ key hi_run (despite having other collision keys).
This uncovered a couple of leftovers from PRs #43846 and #43257 that made few DQM online clients crash (see logs at DQM square).
These are fixed here.

PR validation:

python3 beam_dqm_sourceclient-live_cfg.py runkey=hi_run
python3 beampixel_dqm_sourceclient-live_cfg.py runkey=hi_run
python3 pixellumi_dqm_sourceclient-live_cfg.py runkey=hi_run

all compile fine now.

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:

Not a backport, but will need to be backported to CMSSW_14_0_X for 2024 data-taking purposes.

[cmsbuild]

cms-bot internal usage

[mmusich]

@fabiocos FYI

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44384/39448

• This PR adds an extra 20KB to repository

[cmsbuild]

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

It involves the following packages:

• DQM/Integration (dqm)
• RecoVertex/PrimaryVertexProducer (reconstruction)

@rvenditti, @nothingface0, @antoniovagnerini, @jfernan2, @tjavaid, @cmsbuild, @syuvivida, @mandrenguyen can you please review it and eventually sign? Thanks.
@threus, @fabiocos, @batinkov, @VinInn, @JanFSchulte, @francescobrivio, @rovere, @VourMa, @missirol, @GiacomoSguazzoni, @mtosi, @mmusich, @dgulhan this is something you requested to watch as well.
@antoniovilela, @rappoccio, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

[mmusich]

@cmsbuild, please test

[cmsbuild]

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6365f4/38094/summary.html
COMMIT: c89c5e2
CMSSW: CMSSW_14_1_X_2024-03-12-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/44384/38094/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

• You potentially removed 86 lines from the logs
• Reco comparison results: 51 differences found in the comparisons
• DQMHistoTests: Total files compared: 48
• DQMHistoTests: Total histograms compared: 3297383
• DQMHistoTests: Total failures: 26
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 3297337
• DQMHistoTests: Total skipped: 20
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
• Checked 202 log files, 165 edm output root files, 48 DQM output files
• TriggerResults: no differences found

[fabiocos]

the problem is due to the fact that with runkey=hi_run the logic building the fillDescription in

cmssw/RecoVertex/PrimaryVertexProducer/plugins/PrimaryVertexProducer.cc

Line 445 in c89c5e2

psd0.ifValue(

is selecting from the original offlinePrimaryVertices

    TkClusParameters = cms.PSet(
        TkGapClusParameters = cms.PSet(
            zSeparation = cms.double(1.0)
        ),
        algorithm = cms.string('gap')
    ),

so clearly an attempt to update TkDAClusParameters does not work. So the question is whether the logic that selects the algorithm gap for HI is holding or not. Apparently it is not, if we need this fix. @mandrenguyen any comment about this? Should we just go back to the old situation and give up on cleaning the duplication python/fillDescription?

[mmusich]

So the question is whether the logic that selects the algorithm gap for HI is holding or not.

If we are using the gap algo for offline reco, we certainly can use the same for DQM online (it actually makes more sense to me), but the beamspot people should chime in here as well (@francescobrivio FYI).

[mandrenguyen]

the problem is due to the fact that with runkey=hi_run the logic building the fillDescription in

cmssw/RecoVertex/PrimaryVertexProducer/plugins/PrimaryVertexProducer.cc

Line 445 in c89c5e2

psd0.ifValue(

is selecting from the original offlinePrimaryVertices

    TkClusParameters = cms.PSet(
        TkGapClusParameters = cms.PSet(
            zSeparation = cms.double(1.0)
        ),
        algorithm = cms.string('gap')
    ),

so clearly an attempt to update TkDAClusParameters does not work. So the question is whether the logic that selects the algorithm gap for HI is holding or not. Apparently it is not, if we need this fix. @mandrenguyen any comment about this? Should we just go back to the old situation and give up on cleaning the duplication python/fillDescription?

I'm not 100% sure I understand the question, but let me give some context and maybe this will address it.
The gap algo is in fact needed for vertexing in heavy ions, otherwise we end up with a large amount of PV splitting for a single true collision.
The full set of modifications to the vertexing for pp_on_AA may be found starting from this line (mainly for the benefit of others trying to follow this exchange):

cmssw/RecoVertex/PrimaryVertexProducer/python/OfflinePrimaryVertices_cfi.py

Line 142 in e628a1b

from Configuration.Eras.Modifier_pp_on_XeXe_2017_cff import pp_on_XeXe_2017

[mmusich]

The full set of modifications to the vertexing for pp_on_AA may be found starting from this line (mainly for the benefit of others trying to follow this exchange):

indeed this is what I would naively do as well for the pixel vertices used in online DQM, but the beamspot people (or do we have a dedicated HI beamspot contact ?) need to confirm.

[mandrenguyen]

The full set of modifications to the vertexing for pp_on_AA may be found starting from this line (mainly for the benefit of others trying to follow this exchange):

indeed this is what I would naively do as well for the pixel vertices used in online DQM, but the beamspot people (or do we have a dedicated HI beamspot contact) need to confirm.

From HIN there is just an alca contact (@ashleyahram) but I don't think she'll know how the vertex reco for the beamspot workflow should be configured.
Indeed if someone from the beamspot side can confirm that don't require settings different from the offline heavy-ion ones, I guess that would settle it. At least from the reco/HI side, I think it should be set consistently.

[mmusich]

At least from the reco/HI side, I think it should be set consistently.

notice that these are pixel vertices, so I don't know how appropriate are in general the settings used in offline HI reco for full vertices.

EDIT: in this sense this PR maintains the status quo ante PR #43846

[mmusich]

swapped by mistake d0 with dz

[mmusich]

type bug-fix

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44384/39463

• This PR adds an extra 16KB to repository

[cmsbuild]

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

[mmusich]

@cmsbuild, please test

[cmsbuild]

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6365f4/38112/summary.html
COMMIT: 2542e88
CMSSW: CMSSW_14_1_X_2024-03-13-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/44384/38112/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

• You potentially added 104 lines to the logs
• Reco comparison results: 58 differences found in the comparisons
• DQMHistoTests: Total files compared: 48
• DQMHistoTests: Total histograms compared: 3297383
• DQMHistoTests: Total failures: 9
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 3297354
• DQMHistoTests: Total skipped: 20
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
• Checked 202 log files, 165 edm output root files, 48 DQM output files
• TriggerResults: no differences found

[mandrenguyen]

At least from the reco/HI side, I think it should be set consistently.

notice that these are pixel vertices, so I don't know how appropriate are in general the settings used in offline HI reco for full vertices.

EDIT: in this sense this PR maintains the status quo ante PR #43846

Thanks for pointing that out. The following belongs to the old HI reco, which at some point was the only HI reco we had:
https://github.com/cms-sw/cmssw/blob/master/RecoHI/HiTracking/python/HIPixelAdaptiveVertex_cfi.py
Since the gap algo was used here, it must have also been run in beamspot workflows unless they were restricted to peripheral events, where pp reconstruction could be run.
I think it's fairly safer to always use gap for heavy ions.

[mandrenguyen]

+reconstruction
restores heavy-ion like vertex reconstruction which was unintentionally removed

[tjavaid]

+1

[cmsbuild]

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. @antoniovilela, @rappoccio, @sextonkennedy (and backports should be raised in the release meeting by the corresponding L2)

[antoniovilela]

+1