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

Reduce minYsizeB1 and minYsizeB2 CA cuts for Phase1 #47271

Merged
merged 2 commits into from
Feb 8, 2025

Conversation

bdanzi
Copy link

@bdanzi bdanzi commented Feb 5, 2025

PR description:

The PR aims to minimize the Cluster Size Cuts for the Phase 1 geometry on the B1 and B2 layers, specifically when considering B1-FPIX and B2-FPIX doublets. These doublets serve as the initial inputs for the CA. The changes have been tested on top of the Phase 1 CA-tuned parameters.

This adjustment results in about 5% relative efficiency gain in hltIter0PFlowTrackSelectionHighPurity and a 3% gain in hltMerged. Additionally, it reduces the reliance on the Doublet Recovery iteration, leading to a corresponding decrease in efficiency for hltDoubletRecoveryPFlowTrackSelectionHighPurity. Track resolution shows a small improvement as a function of pT.

PR validation:

600 events TTBarPU 2025 (/RelValTTbar_14TeV/CMSSW_14_2_1-PU_142X_mcRun3_2025_realistic_v4_STD_Winter25_PU-v1/GEN-SIM-DIGI-RAW):
http://uaf-3.t2.ucsd.edu/~bdanzi/PR_2025/plots_hlt.html
500 events TTBarPU EEOR3 (/RelValTTbar_14TeV/CMSSW_14_0_9-PU_140X_mcRun3_2024_realistic_EOR3_TkDPGv7_RV245_2024-v2/GEN-SIM-DIGI-RAW):
http://uaf-3.t2.ucsd.edu/~bdanzi/PRClusterSize_EEOR3_TTBarPU/plots_hlt.html

Instructions on how to run

Tested in CMSSW_15_0_0_pre2 at HLT step.

Generate the .cff file on lxplus and place it inside HLTrigger/Configuration/python by running:

hltGetConfiguration /dev/CMSSW_14_2_0/GRun/V10 --globaltag 140X_mcRun3_2024_realistic_EOR3_TkDPGv7 --mc --unprescale --output minimal --max-events 500 --input file:input_file.root --cff --eras Run3_2024 --l1-emulator uGT --l1 L1Menu_Collisions2024_v1_3_0_xml --paths MC_ReducedIterativeTracking_v22 > hltMC_default_onlyTracking.py

The .cff should be loaded to run the following script:

import FWCore.ParameterSet.Config as cms
from Configuration.Eras.Era_Run3_2024_cff import Run3_2024
process = cms.Process( "HLTX", Run3_2024)

process.load("Configuration.StandardSequences.Accelerators_cff")
# import of standard configurations
process.load('Configuration.StandardSequences.Services_cff')
process.load('SimGeneral.HepPDTESSource.pythiapdt_cfi')
process.load('FWCore.MessageService.MessageLogger_cfi')
process.load('Configuration.EventContent.EventContent_cff')
process.load('SimGeneral.MixingModule.mixNoPU_cfi')
process.load('Configuration.StandardSequences.GeometryRecoDB_cff')
process.load('Configuration.StandardSequences.MagneticField_cff')
process.load('HLTrigger.Configuration.hltMC_default_onlyTracking')
process.load('Configuration.StandardSequences.Validation_cff')
process.load('Configuration.StandardSequences.FrontierConditions_GlobalTag_cff')

process.maxEvents = cms.untracked.PSet(
    input = cms.untracked.int32(-1),
    output = cms.optional.untracked.allowed(cms.int32,cms.PSet)
)

# Input source
process.source = cms.Source("PoolSource",
    fileNames = cms.untracked.vstring(
    'file:input_file.root'
    ),
    secondaryFileNames = cms.untracked.vstring()
)

process.options = cms.untracked.PSet(
    IgnoreCompletely = cms.untracked.vstring(),
    Rethrow = cms.untracked.vstring(),
    TryToContinue = cms.untracked.vstring(),
    accelerators = cms.untracked.vstring('*'),
    allowUnscheduled = cms.obsolete.untracked.bool,
    canDeleteEarly = cms.untracked.vstring(),
    deleteNonConsumedUnscheduledModules = cms.untracked.bool(True),
    dumpOptions = cms.untracked.bool(False),
    emptyRunLumiMode = cms.obsolete.untracked.string,
    eventSetup = cms.untracked.PSet(
        forceNumberOfConcurrentIOVs = cms.untracked.PSet(
            allowAnyLabel_=cms.required.untracked.uint32
        ),
        numberOfConcurrentIOVs = cms.untracked.uint32(0)
    ),
    fileMode = cms.untracked.string('FULLMERGE'),
    forceEventSetupCacheClearOnNewRun = cms.untracked.bool(False),
    holdsReferencesToDeleteEarly = cms.untracked.VPSet(),
    makeTriggerResults = cms.obsolete.untracked.bool,
    modulesToCallForTryToContinue = cms.untracked.vstring(),
    modulesToIgnoreForDeleteEarly = cms.untracked.vstring(),
    numberOfConcurrentLuminosityBlocks = cms.untracked.uint32(0),
    numberOfConcurrentRuns = cms.untracked.uint32(1),
    numberOfStreams = cms.untracked.uint32(0),
    numberOfThreads = cms.untracked.uint32(8),
    printDependencies = cms.untracked.bool(False),
    sizeOfStackForThreadsInKB = cms.optional.untracked.uint32,
    throwIfIllegalParameter = cms.untracked.bool(True),
    wantSummary = cms.untracked.bool(False)
)

# Production Info
process.configurationMetadata = cms.untracked.PSet(
    annotation = cms.untracked.string('step2 nevts:10'),
    name = cms.untracked.string('Applications'),
    version = cms.untracked.string('$Revision: 1.19 $')
)

# Output definition

process.DQMoutput = cms.OutputModule("DQMRootOutputModule",
    dataset = cms.untracked.PSet(
        dataTier = cms.untracked.string('DQMIO'),
        filterName = cms.untracked.string('')
    ),
    fileName = cms.untracked.string('step2_HLT_VALIDATION_paramTuned.root'),
    outputCommands = process.DQMEventContent.outputCommands,
    splitLevel = cms.untracked.int32(0)
)

# Additional output definition

# Other statements
from HLTrigger.Configuration.CustomConfigs import ProcessName
process = ProcessName(process)

from Configuration.Applications.ConfigBuilder import ConfigBuilder
process.hltMultiTrackValidation.visit(ConfigBuilder.MassSearchReplaceProcessNameVisitor("HLT", "HLTX", whitelist = ("subSystemFolder",), verbose = False))
process.mix.playback = True
process.mix.digitizers = cms.PSet()
for a in process.aliases: delattr(process, a)
process.RandomNumberGeneratorService.restoreStateLabel=cms.untracked.string("randomEngineStateProducer")
from Configuration.AlCa.GlobalTag import GlobalTag
process.GlobalTag = GlobalTag(process.GlobalTag, '140X_mcRun3_2024_realistic_EOR3_TkDPGv7', '')
process.hltTrackValidator.label = ["hltPixelTracks", "hltIter0PFlowCtfWithMaterialTracks", "hltIter0PFlowTrackSelectionHighPurity", "hltMergedTracks","hltDoubletRecoveryPFlowTrackSelectionHighPurity"]
# Path and EndPath definitions
process.validation_step = cms.EndPath(process.hltMultiTrackValidation)
process.DQMoutput_step = cms.EndPath(process.DQMoutput)

process.hltSiStripRawToClustersFacility.onDemand = cms.bool(False)
process.hltPixelTracksSoA.CAThetaCutBarrel = cms.double(0.00111685053)
process.hltPixelTracksSoA.CAThetaCutForward = cms.double(0.00249872683)
process.hltPixelTracksSoA.hardCurvCut =  cms.double(0.695091509)
process.hltPixelTracksSoA.dcaCutInnerTriplet = cms.double(0.0419242041)
process.hltPixelTracksSoA.dcaCutOuterTriplet = cms.double(0.293522194)
process.hltPixelTracksSoA.phiCuts = cms.vint32(
    832, 379, 481, 765, 1136,
    706, 656, 407, 1212, 404,
    699, 470, 652, 621, 1017,
    616, 450, 555, 572
)
# Schedule definition
# process.schedule imported from cff in HLTrigger.Configuration
process.schedule.extend([process.validation_step,process.DQMoutput_step])
from PhysicsTools.PatAlgos.tools.helpers import associatePatAlgosToolsTask
associatePatAlgosToolsTask(process)

# customisation of the process.

# Automatic addition of the customisation function from HLTrigger.Configuration.customizeHLTforMC
from HLTrigger.Configuration.customizeHLTforMC import customizeHLTforMC 

#call to customisation function customizeHLTforMC imported from HLTrigger.Configuration.customizeHLTforMC
process = customizeHLTforMC(process)

# Automatic addition of the customisation function from SimGeneral.MixingModule.fullMixCustomize_cff
from SimGeneral.MixingModule.fullMixCustomize_cff import setCrossingFrameOn 

#call to customisation function setCrossingFrameOn imported from SimGeneral.MixingModule.fullMixCustomize_cff
process = setCrossingFrameOn(process)

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 5, 2025

cms-bot internal usage

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 5, 2025

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47271/43578

  • There are other open Pull requests which might conflict with changes you have proposed:

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 5, 2025

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

It involves the following packages:

  • Geometry/CommonTopologies (geometry)

@Dr15Jones, @bsunanda, @civanch, @cmsbuild, @kpedro88, @makortel, @mdhildreth can you please review it and eventually sign? Thanks.
@VinInn, @VourMa, @bsunanda, @fabiocos, @martinamalberti, @mmusich, @mtosi 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

static constexpr int minYsizeB1 = 36;
static constexpr int minYsizeB2 = 28;
static constexpr int minYsizeB1 = 1;
static constexpr int minYsizeB2 = 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

could these parameters be exposed as configurable parameters instead of hardcoded?
I guess this would ease the checks to be asked to various stakeholders.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should be done in ef9d50d

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks!

@mmasciov
Copy link
Contributor

mmasciov commented Feb 5, 2025

type tracking

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 5, 2025

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47271/43582

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 5, 2025

Pull request #47271 was updated. @Dr15Jones, @bsunanda, @civanch, @cmsbuild, @jfernan2, @kpedro88, @makortel, @mandrenguyen, @mdhildreth can you please check and sign again.

@mmusich
Copy link
Contributor

mmusich commented Feb 5, 2025

@cmsbuild, please test

  • squashing the commits before merge would be nice ;)

@@ -197,6 +199,9 @@ void CAHitNtupletGeneratorOnGPU<pixelTopology::Phase1>::fillDescriptions(edm::Pa
trackQualityCuts.add<double>("quadrupletMaxTip", 0.5)->setComment("Max |Tip| for quadruplets, in cm");
trackQualityCuts.add<double>("quadrupletMaxZip", 12.)->setComment("Max |Zip| for quadruplets, in cm");

desc.add<int>("minYsizeB1", 1)->setComment("Min Y cluster size in pixel B1");
desc.add<int>("minYsizeB2", 1)->setComment("Min Y cluster size in pixel B2");
Copy link
Contributor

@mmusich mmusich Feb 5, 2025

Choose a reason for hiding this comment

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

@cms-sw/orp-l2, it would be great to get this PR into CMSSW_15_0_0 for 2025 HLT development purposes.
As it is, it does change (HLT) physics, could it enter in pre3 still?
Otherwise I guess it would be more acceptable if it would just make the parameters configurable, followed by a change in the HLT menu configuration?

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's try to get it merged tomorrow, that will be fine for pre3

Copy link
Contributor

Choose a reason for hiding this comment

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

excellent, thanks.

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like for some reason the commands to enable gpu and test are not being received correctly?

Copy link
Contributor

Choose a reason for hiding this comment

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

It now seems to have worked, although tests are still pending - I see a similar behavior in other PRs

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 5, 2025

+1

Size: This PR adds an extra 40KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-32b45b/44229/summary.html
COMMIT: 4689225
CMSSW: CMSSW_15_0_X_2025-02-05-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/47271/44229/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: 51
  • DQMHistoTests: Total histograms compared: 4130882
  • DQMHistoTests: Total failures: 25918
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 4104944
  • DQMHistoTests: Total skipped: 20
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 50 files compared)
  • Checked 222 log files, 194 edm output root files, 51 DQM output files
  • TriggerResults: found differences in 9 / 49 workflows

@cmsbuild cmsbuild modified the milestones: CMSSW_15_1_X, CMSSW_15_0_X Feb 7, 2025
@mmusich
Copy link
Contributor

mmusich commented Feb 7, 2025

Milestone for this pull request has been moved to CMSSW_15_1_X. Please open a backport if it should also go in to CMSSW_15_0_X.

@bdanzi @mmasciov as it looks like we didn't make it for pre3, please open a backport to CMSSW_15_0_X as well. Thank you.

@cmsbuild cmsbuild modified the milestones: CMSSW_15_0_X, CMSSW_15_1_X Feb 7, 2025
@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 7, 2025

+1

Size: This PR adds an extra 40KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-32b45b/44236/summary.html
COMMIT: 34fd1de
CMSSW: CMSSW_15_0_X_2025-02-05-2300/el8_amd64_gcc12
Additional Tests: GPU
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/47271/44236/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

GPU Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 24 differences found in the comparisons
  • DQMHistoTests: Total files compared: 7
  • DQMHistoTests: Total histograms compared: 53071
  • DQMHistoTests: Total failures: 901
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 52170
  • DQMHistoTests: Total skipped: 0
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 6 files compared)
  • Checked 24 log files, 30 edm output root files, 7 DQM output files
  • TriggerResults: no differences found

@mmasciov
Copy link
Contributor

mmasciov commented Feb 7, 2025

As it was a general issue, I would have hoped it would be taken into account by @cms-sw/orp-l2, especially after the comment by @mandrenguyen, as this would have been fully tested and signed if it wasn't for the issue described above by @smuzaffar.
But sure, we will open a backport at this point.

@mandrenguyen
Copy link
Contributor

As it was a general issue, I would have hoped it would be taken into account by @cms-sw/orp-l2, especially after the comment by @mandrenguyen, as this would have been fully tested and signed if it wasn't for the issue described above by @smuzaffar. But sure, we will open a backport at this point.

We did take the request into account, as you just noted. We decided, however, not to further the delay the pre-release which was scheduled for Tuesday. The final 15_0_0 will not be cut until the 24th. This will be included.

@mmusich
Copy link
Contributor

mmusich commented Feb 7, 2025

The final 15_0_0 will not be cut until the 24th. This will be included.

Thanks. As far as I can tell the main inconvenience is that if this would have entered in pre3 we could have profited of the regular HLT release validation (one pre-release earlier).

@bdanzi
Copy link
Author

bdanzi commented Feb 7, 2025

Milestone for this pull request has been moved to CMSSW_15_1_X. Please open a backport if it should also go in to CMSSW_15_0_X.

@bdanzi @mmasciov as it looks like we didn't make it for pre3, please open a backport to CMSSW_15_0_X as well. Thank you.

Done it #47296 .

@bdanzi bdanzi closed this Feb 7, 2025
@bdanzi bdanzi reopened this Feb 7, 2025
@jfernan2
Copy link
Contributor

jfernan2 commented Feb 7, 2025

+1

@mmusich
Copy link
Contributor

mmusich commented Feb 7, 2025

@cms-sw/geometry-l2 can you please take a look to this PR and the corresponding backport #47296 ? We'll need those for the 2025 HLT menu development.

@civanch
Copy link
Contributor

civanch commented Feb 8, 2025

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 8, 2025

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

@mandrenguyen
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit d33ec79 into cms-sw:master Feb 8, 2025
26 checks passed
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.

8 participants