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

HFNose: ticl updates #33001

Merged
merged 14 commits into from
Mar 22, 2021
Merged

HFNose: ticl updates #33001

merged 14 commits into from
Mar 22, 2021

Conversation

mariadalfonso
Copy link
Contributor

@mariadalfonso mariadalfonso commented Feb 25, 2021

  • customize the ticl parameters after the ticl3 update
  • fix rhtools_.getPositionLayer recently added
  • adds EM track seeded and Hadronic iterations
  • add caloParticles

updates only on HFnose, no changes expected on HGCAL

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33001/21273

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

@mariadalfonso mariadalfonso changed the title Ticl nosefixes HFNose: ticl updates Feb 25, 2021
@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33001/21277

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

RecoHGCal/Configuration
RecoHGCal/TICL
SimGeneral/MixingModule

@perrotta, @civanch, @kpedro88, @cmsbuild, @srimanob, @mdhildreth, @slava77, @jpata can you please review it and eventually sign? Thanks.
@makortel, @felicepantaleo, @rovere, @apsallid, @sobhatta, @clelange, @lecriste, @hatakeyamak, @fabiocos, @slomeo this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

RecoHGCal/TICL/plugins/SeedingRegionByTracks.h Outdated Show resolved Hide resolved
filtered_mask = "filteredLayerClustersHFNoseEM:EMn",
seeding_regions = "ticlSeedingGlobalHFNose",
time_layerclusters = "hgcalLayerClustersHFNose:timeLayerCluster",
min_layers_per_trackster = 6
itername = "EMn",
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you use a more expressive name? Also for the downstream HADn and TrkEMn.

@@ -1,6 +1,6 @@
import FWCore.ParameterSet.Config as cms

from RecoHGCal.TICL.TICLSeedingRegions_cff import ticlSeedingGlobal
from RecoHGCal.TICL.TICLSeedingRegions_cff import ticlSeedingGlobal, ticlSeedingGlobalHFNose, ticlSeedingForHFHFNose
Copy link
Contributor

Choose a reason for hiding this comment

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

I cannot find where ticlSeedingForHFHFNose is defined.
Possibly you meant ticlSeedingTrkHFNose?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the test failed because the ticlSeedingForHFHFNose wasn't committed; I did now.
ticlSeedingByHFHFNose allows to seed from a HFcell; but the iteration is not build as would be better to build the trackster outside-in.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for the hadrons I foresee 3 set of iterations:

  1. early shower for charger particle: track seeded + short trackster
  2. late shower: HFseeded with outside-in trackster
  3. all the rest will be taken from the self seeded

@rovere
Copy link
Contributor

rovere commented Feb 27, 2021

@mariadalfonso Out of curiosity, how did you solve the propagation to HFNose business?

@rovere
Copy link
Contributor

rovere commented Feb 27, 2021

@cmsbuild please test

@cmsbuild
Copy link
Contributor

-1

Failed Tests: RelVals RelVals-INPUT AddOn
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-547c7c/13140/summary.html
COMMIT: e453f78
CMSSW: CMSSW_11_3_X_2021-02-26-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33001/13140/install.sh to create a dev area with all the needed externals and cmssw changes.

  • DAS Queries: The DAS query tests failed, see the summary page for details.

RelVals

  • 5.15.1_TTbar+TTbarFS+HARVESTFS/step1_TTbar+TTbarFS+HARVESTFS.log
  • 135.4135.4_ZEE_13+ZEEFS_13+HARVESTUP15FS+MINIAODMCUP15FS/step1_ZEE_13+ZEEFS_13+HARVESTUP15FS+MINIAODMCUP15FS.log
  • 140.56140.56_RunHI2018+RunHI2018+RECOHID18+HARVESTDHI18/step2_RunHI2018+RunHI2018+RECOHID18+HARVESTDHI18.log
Expand to see more relval errors ...

RelVals-INPUT

  • 4.64.6_MinimumBias2010A+MinimumBias2010A+RECOSKIMALCA+HARVESTDR1/step2_MinimumBias2010A+MinimumBias2010A+RECOSKIMALCA+HARVESTDR1.log
  • 140.54140.54_RunPA2013+RunPA2013+RECO_PPbData+HARVEST_PPbData/step2_RunPA2013+RunPA2013+RECO_PPbData+HARVEST_PPbData.log
  • 140.56140.56_RunHI2018+RunHI2018+RECOHID18+HARVESTDHI18/step2_RunHI2018+RunHI2018+RECOHID18+HARVESTDHI18.log
Expand to see more relval errors ...

AddOn Tests

  • fastsimcmsDriver.py TTbar_8TeV_TuneCUETP8M1_cfi --conditions auto:run1_mc --fast -n 100 --eventcontent AODSIM,DQM --relval 100000,1000 -s GEN,SIM,RECOBEFMIX,DIGI:pdigi_valid,L1,DIGI2RAW,L1Reco,RECO,EI,VALIDATION --customise=HLTrigger/Configuration/CustomConfigs.L1THLT --datatier GEN-SIM-DIGI-RECO,DQMIO --beamspot Realistic8TeVCollision : FAILED - time: date Sat Feb 27 19:52:49 2021-date Sat Feb 27 19:52:40 2021 s - exit: 256
  • hlt_data_Fake2cmsDriver.py RelVal -s HLT:Fake2,RAW2DIGI,L1Reco,RECO --data --scenario=pp -n 10 --conditions auto:run2_data_Fake2 --relval 9000,50 --datatier "RAW-HLT-RECO" --eventcontent FEVTDEBUGHLT --customise=HLTrigger/Configuration/CustomConfigs.L1THLT --era Run2_2016 --processName=HLTRECO --filein file:RelVal_Raw_Fake2_DATA.root --fileout file:RelVal_Raw_Fake2_DATA_HLT_RECO.root : FAILED - time: date Sat Feb 27 20:19:52 2021-date Sat Feb 27 19:52:44 2021 s - exit: 256
  • hlt_data_PRefcmsDriver.py RelVal -s HLT:PRef,RAW2DIGI,L1Reco,RECO --data --scenario=pp -n 10 --conditions auto:run3_data_PRef --relval 9000,50 --datatier "RAW-HLT-RECO" --eventcontent FEVTDEBUGHLT --customise=HLTrigger/Configuration/CustomConfigs.L1THLT --era Run3 --processName=HLTRECO --filein file:RelVal_Raw_PRef_DATA.root --fileout file:RelVal_Raw_PRef_DATA_HLT_RECO.root : FAILED - time: date Sat Feb 27 20:18:14 2021-date Sat Feb 27 19:52:48 2021 s - exit: 256
Expand to see more addon errors ...

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-547c7c/13506/summary.html
COMMIT: b1df50f
CMSSW: CMSSW_11_3_X_2021-03-14-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33001/13506/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: 5 differences found in the comparisons
  • DQMHistoTests: Total files compared: 37
  • DQMHistoTests: Total histograms compared: 2635087
  • DQMHistoTests: Total failures: 103
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2634962
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 36 files compared)
  • Checked 155 log files, 37 edm output root files, 37 DQM output files

@jpata
Copy link
Contributor

jpata commented Mar 15, 2021

Thanks for the updates. Looks like tests run now, no more crash in the validator. There are some HGCAL reco and DQM differences. @mariadalfonso et al, do they look OK to you?

@mariadalfonso
Copy link
Contributor Author

Thanks for the updates. Looks like tests run now, no more crash in the validator. There are some HGCAL reco and DQM differences. @mariadalfonso et al, do they look OK to you?

https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_11_3_X_2021-03-12-1100+547c7c/41671/validateJR/all_OldVSNew_TTbar14TeV2026D60wf28234p0/all_OldVSNew_TTbar14TeV2026D60wf28234p0c_recoPFClusters_particleFlowClusterHGCal__RECO_obj_eta.png

The few differences on HGCal are related to the particleFlowClusterHGCal. This particleFlowClusterHGCal (likely) depends on the caloParticles change as I suspect the particleFlowClusterHGCal still depend on the sim-assisted reco.
The validation package now doesn't show the differences of the HFnose (eta 3-4.2) because of hardcoded values in Validation/HGCalValidation.

@jfernan2
Copy link
Contributor

And any idea why these differences appear only in wf 28234.0 and not in the rest of Phase2 samples?
Thanks

@mariadalfonso
Copy link
Contributor Author

And any idea why these differences appear only in wf 28234.0 and not in the rest of Phase2 samples?
Thanks

yes, the HFnose is only in the WF28234.0 ( as HFnose it's not in the approved Phase2 detector for Run4 )

the phase2_hfnose era activate the caloParticles here
https://github.com/cms-sw/cmssw/pull/33001/files#diff-969698d33dc7e57ea0dce87e527740aa866cc4c22da5216aaa83e740ec617b65R38

@jfernan2
Copy link
Contributor

+1

@jpata
Copy link
Contributor

jpata commented Mar 17, 2021

+reconstruction

  • TICL updates for HFNose
  • reco changes only in the Phase2 workflow 28234.0 which contains HFNose, as expected

@mariadalfonso
Copy link
Contributor Author

@civanch, @kpedro88, @srimanob

would be great if we can get this in the 11_3_pre5 for this Tuesday

@civanch
Copy link
Contributor

civanch commented Mar 21, 2021

+1

@srimanob
Copy link
Contributor

+Upgrade

@cmsbuild
Copy link
Contributor

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

@silviodonato
Copy link
Contributor

+1

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.

10 participants