[12_4_X] Fix Pixel Clusters for Phase2 HLT Menu

[AdrianoDee]

PR description:

A simple fix to siPixelClusters constants. Backport to 12_4_X of #39323.

PR validation:

See #39323. Tracking efficiency recovered.

[cmsbuild]

A new Pull Request was created by @AdrianoDee for CMSSW_12_4_X.

It involves the following packages:

• HLTrigger/Configuration (hlt)

@cmsbuild, @missirol, @Martin-Grunewald can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @missirol, @silviodonato this is something you requested to watch as well.
@perrotta, @dpiparo, @rappoccio you are the release manager for this.

cms-bot commands are listed here

• Backported from Fix Pixel Clusters for Phase2 HLT Menu #39323

[AdrianoDee]

type bugfix

[AdrianoDee]

test parameters:

• workflows = 39434.75
• relvals_opt = --what cleanedupgrade,standard,highstats,pileup,generator,extendedgen,production,ged,machine,premix

[AdrianoDee]

please test

[Martin-Grunewald]

backort of #39323

[Martin-Grunewald]

backport of #39323

[cmsbuild]

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-78a56f/27392/summary.html
COMMIT: 3ad67f0
CMSSW: CMSSW_12_4_X_2022-09-06-2300/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/39330/27392/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: 3677280
• DQMHistoTests: Total failures: 8
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 3677250
• 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: found differences in 1 / 49 workflows

[missirol]

+hlt

[cmsbuild]

This pull request is fully signed and it will be integrated in one of the next CMSSW_12_4_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_12_6_X is complete. This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @rappoccio (and backports should be raised in the release meeting by the corresponding L2)

[missirol]

hold

This is a bugfix to the HLT Phase-2 menu, which changes its physics outputs.

@cms-sw/orp-l2 @cms-sw/upgrade-l2 , since 12_4_X is closed for development, do you think this could/should still be merged in 12_4_X?

[cmsbuild]

Pull request has been put on hold by @missirol
They need to issue an unhold command to remove the hold state or L1 can unhold it for all

[perrotta]

hold

This is a bugfix to the HLT Phase-2 menu, which changes its physics outputs.

@cms-sw/orp-l2 @cms-sw/upgrade-l2 , since 12_4_X is closed for development, do you think this could/should still be merged in 12_4_X?

@missirol we are still waiting for the last additions for the MC production release, that has to be in 12_4_X. A few of them will change the physics output, then for what us release managers are concerned, it is not an issue backporting this bug fix which improves Tracking HLT efficiency.
Of course, the effect on run3 data and MC must be fully understood and validated: can you please confirm that this is actually the case?

[missirol]

@perrotta , thanks for the clarification.

This fix is a configuration change inside the HLT Phase-2 menu, so it has (by construction) no impact on anything related to Run 3. In that sense, I think it is okay to merge.

[perrotta]

@perrotta , thanks for the clarification.

This fix is a configuration change inside the HLT Phase-2 menu, so it has (by construction) no impact on anything related to Run 3. In that sense, I think it is okay to merge.

You are right, I got confused by another PR that had Phase2 in the name, but was reported at the ORP as affecting also Run3.
As it only touches Phase2 HLT menu, I wonder why do you want to backport it in 12_4: samples for trigger studies in Phase2 are expected to be produced with 12_5. Do you need this for some private study and development? Were there any Phase2 productions already done with 12_4? I think to remember none was...

[missirol]

@AdrianoDee (or @trtomei or @fwyzard) can comment better. My understanding is that the HLT Phase-2 group currently uses 12_4_X for studies on MC samples that were produced in 12_3_X.

In my understanding, it would also be okay for them to leave the PR here (so developers can use the branch) without merging it in 12_4_X.

[AdrianoDee]

Indeed the group is working in 12_4_X and in general we would live (at least for the moment) with this PR staying unmerged and at disposal of the developers (as is happening and will happen with other developments).

Nevertheless, given this is a quite small fix I'd say this could be considered an exception. But I leave this judgment to ORP.

[missirol]

unhold

To be clear, I don't mean to hold this PR. I just wanted to ask first about the policy for backports to 12_4_X.

[cmsbuild]

This pull request is fully signed and it will be integrated in one of the next CMSSW_12_4_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_12_6_X is complete. This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @rappoccio (and backports should be raised in the release meeting by the corresponding L2)

[rappoccio]

Let's discuss at ORP on Tuesday.

[missirol]

Let's discuss at ORP on Tuesday.

Since there is no ORP meeting this week, what's the plan for this PR?

I understood that there wasn't any large-scale MC production in 12_4_X for Phase-2, so integrating this fix would not create an inconsistency with existing MC samples.

Are there other criteria?

What do others prefer?

[rovere]

Ciao,
to be on the safe side: this PR is relevant for HLT studies for Phase2, while it is definitely not needed/useful for data taking.
Since, at some point, we will need a new release for data-taking after the TS1 and LHC recovery, I will not consider this PR as needed (for that specific purpose).

Scream if that's not correct.

Marco (as ORM of the week).

[srimanob]

I assume one can use customize_command when rerun HLT in 12_4 on 12_3 RAW if this PR does not merge.

[fwyzard]

I assume one can use customize_command when rerun HLT in 12_4 on 12_3 RAW if this PR does not merge.

Do you mean as part of the central production ?

Probably... but then it would be much cleaner to just merge this PR with the bugfix.

[rovere]

To clarify: I was not arguing against having this merged.
I wanted to have a confirmation that it is not strictly needed for data-taking and that it won't have any undesired side effects on that.

Once that is said, merging it is the cleanest solution, IMHO.

[fwyzard]

It does not impact data taking.

[srimanob]

I assume one can use customize_command when rerun HLT in 12_4 on 12_3 RAW if this PR does not merge.

Do you mean as part of the central production ?

Probably... but then it would be much cleaner to just merge this PR with the bugfix.

Nope. I don't think we will rerun HLT step in central production. I understand that HLT needs to rerun HLT on RAW several times when doing study. This happens privately.

[rovere]

Ciao @srimanob even if reprocessing happens privately, what is the meaning of having a 12_4_X release with an HLT for Phase2 that is knowingly broken when we have the fix?

[srimanob]

Ciao @srimanob even if reprocessing happens privately, what is the meaning of having a 12_4_X release with an HLT for Phase2 that is knowingly broken when we have the fix?

I does not mean against merging. I just mean we can survive without merging. Merging will make more clean driver, and I support it.

[fwyzard]

I don't think we will rerun HLT step in central production.

Ah... I didn't know that.

Why not ?

On the other hand, do we foresee any Phase-2 production in 12.4.x ? I understood the next one will be in 12.5.x.

[fwyzard]

I just mean we can survive without merging.

Well, yes, but that is technically true for every single PR - people can always check them out locally and rebuild a whole release...

[perrotta]

Trying to converge: according to your discussion I don't see possible practical counterindications in merging this, as it would be the case if any official Phase2 production with HLT was already started in 12_4_X.

If there are no objections during the next hours (24?), we can merge this one. Please reply only if you have objections, (and to reply to them eventually, of course)

[srimanob]

I don't think we will rerun HLT step in central production.
Ah... I didn't know that.
Why not ?

I assume it is quick enough to rerun HLT privately and use existing RECO output, and current output is RAW-MINIAOD. Does HLT step run once, or need to rerun several times, i.e. tuning?

On the other hand, do we foresee any Phase-2 production in 12.4.x ? I understood the next one will be in 12.5.x.

We don't foresee any Phase-2 production in 12_4 as far as I know. No sign to make campaign.
The next one is 12_5 for L1T emulation.

[perrotta]

+1