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

MTD reconstruction: revision of the AODSIM event content #34195

Merged
merged 8 commits into from Jun 25, 2021

Conversation

fabiocos
Copy link
Contributor

PR description:

Revision of the AODSIM event content of MTD, reducing effectively the output size for PU 200 by ~ 1/3. The ValueMaps kept in the events all refer now to the associated generalTrack, and two more are added to allow the MVA quality flag calculation to be performed without the need of a saved recoTrackExtras.
See this presentation at the MTD DPG meeting for further details.

PR validation:

Test wf. 34634.0 runs smoothly, the update of the output is as expected, and the DQM comparison with the existing workflow does not show differences (as of CMSSW_12_0_0_pre2).

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34195/23411

  • This PR adds an extra 32KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @fabiocos (Fabio Cossutti) for master.

It involves the following packages:

RecoLocalFastTime/Configuration
RecoMTD/Configuration
RecoMTD/TimingIDTools
RecoMTD/TrackExtender
Validation/MtdValidation

@perrotta, @andrius-k, @kmaeshima, @ErnestaP, @kpedro88, @cmsbuild, @srimanob, @jfernan2, @ahmad3213, @slava77, @jpata, @rvenditti can you please review it and eventually sign? Thanks.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

@srimanob
Copy link
Contributor

Thanks @fabiocos

@fabiocos
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1018cd/16130/summary.html
COMMIT: f7fa938
CMSSW: CMSSW_12_0_X_2021-06-20-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/34195/16130/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
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 16 differences found in the comparisons
  • DQMHistoTests: Total files compared: 38
  • DQMHistoTests: Total histograms compared: 2785631
  • DQMHistoTests: Total failures: 12
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 2785596
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -0.004 KiB( 37 files compared)
  • DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
  • Checked 160 log files, 37 edm output root files, 38 DQM output files
  • TriggerResults: no differences found

@jfernan2
Copy link
Contributor

+1

@fabiocos
Copy link
Contributor Author

fabiocos commented Jun 22, 2021

15:43 farmui01 816> diff -b cont3 /gpfs/cms/users/cossutti/Timing/event/std/CMSSW_12_0_0_pre2/work/prova/cont3                                                                                                                                                                                                                
19a20
> edm::ValueMap<float>                  "trackExtenderWithMTD"      "pathLength"      "RECO"    
24a26
> edm::ValueMap<float>                  "trackExtenderWithMTD"      "sigmatmtd"       "RECO"    
26a29
> edm::ValueMap<float>                  "trackExtenderWithMTD"      "tmtd"            "RECO"    
28,29d30
< edm::ValueMap<int>                    "trackExtenderWithMTD"      "npixBarrel"      "RECO"    
< edm::ValueMap<int>                    "trackExtenderWithMTD"      "npixEndcap"      "RECO"    
15:44 farmui01 817> diff -b reco_cont3 /gpfs/cms/users/cossutti/Timing/event/std/CMSSW_12_0_0_pre2/work/prova/reco_cont3                                                                                                                                                                                                      
17a18
> edm::ValueMap<float>                  "trackExtenderWithMTD"      "pathLength"      "RECO"    
22a24
> edm::ValueMap<float>                  "trackExtenderWithMTD"      "sigmatmtd"       "RECO"    
24a27
> edm::ValueMap<float>                  "trackExtenderWithMTD"      "tmtd"            "RECO"    
26,27d28
< edm::ValueMap<int>                    "trackExtenderWithMTD"      "npixBarrel"      "RECO"    
< edm::ValueMap<int>                    "trackExtenderWithMTD"      "npixEndcap"      "RECO"    
15:44 farmui01 818> diff -b aod_cont3 /gpfs/cms/users/cossutti/Timing/event/std/CMSSW_12_0_0_pre2/work/prova/aod_cont3                                                                                                                                                                                                        
0a1
> edm::OwnVector<TrackingRecHit,edm::ClonePolicy<TrackingRecHit> >    "trackExtenderWithMTD"      ""                "RECO"    
12a14
> edm::ValueMap<float>                  "trackExtenderWithMTD"      "pathLength"      "RECO"    
17a20
> edm::ValueMap<float>                  "trackExtenderWithMTD"      "sigmatmtd"       "RECO"    
19a23
> edm::ValueMap<float>                  "trackExtenderWithMTD"      "tmtd"            "RECO"    
21,22c25,28
< edm::ValueMap<int>                    "trackExtenderWithMTD"      "npixBarrel"      "RECO"    
< edm::ValueMap<int>                    "trackExtenderWithMTD"      "npixEndcap"      "RECO"    
---
> edmNew::DetSetVector<FTLCluster>      "mtdClusters"               "FTLBarrel"       "RECO"    
> edmNew::DetSetVector<FTLCluster>      "mtdClusters"               "FTLEndcap"       "RECO"    
> vector<reco::Track>                   "trackExtenderWithMTD"      ""                "RECO"    
> vector<reco::TrackExtra>              "trackExtenderWithMTD"      ""                "RECO"    

@perrotta @jpata the last version should match your favoured style, the output is what expected (see above). At the end of the day it is mostly a matter of taste.

@jpata
Copy link
Contributor

jpata commented Jun 23, 2021

@cmsbuild please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1018cd/16183/summary.html
COMMIT: 78321d5
CMSSW: CMSSW_12_0_X_2021-06-22-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/34195/16183/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
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 12 differences found in the comparisons
  • DQMHistoTests: Total files compared: 38
  • DQMHistoTests: Total histograms compared: 2785631
  • DQMHistoTests: Total failures: 7
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2785602
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 37 files compared)
  • Checked 160 log files, 37 edm output root files, 38 DQM output files
  • TriggerResults: no differences found

@jpata
Copy link
Contributor

jpata commented Jun 23, 2021

Here's what we see in 23434.21/step3.root using compareProducts (did it manually now, but enable profiling could have been used in retrospect)

-----------------------------------------------------------------
   or, B         new, B      delta, B   delta, %   deltaJ, %    branch 
-----------------------------------------------------------------
 552634.0 ->         0.0    -552634     OLDO  -3.89     TrackingRecHitsOwned_trackExtenderWithMTD__RECO.
 108233.0 ->         0.0    -108233     OLDO  -0.76     FTLClusteredmNewDetSetVector_mtdClusters_FTLEndcap_RECO.
 755745.0 ->         0.0    -755745     OLDO  -5.32     recoTracks_trackExtenderWithMTD__RECO.
      0.0 ->      2131.8       2132     NEWO   0.01     intedmValueMap_trackExtenderWithMTD_npixBarrel_RECO.
  13092.4 ->         0.0     -13092     OLDO  -0.09     floatedmValueMap_trackExtenderWithMTD_pathLength_RECO.
   5726.1 ->         0.0      -5726     OLDO  -0.04     floatedmValueMap_trackExtenderWithMTD_sigmatmtd_RECO.
 136952.0 ->         0.0    -136952     OLDO  -0.96     FTLClusteredmNewDetSetVector_mtdClusters_FTLBarrel_RECO.
      0.0 ->      4267.0       4267     NEWO   0.03     intedmValueMap_trackExtenderWithMTD_npixEndcap_RECO.
   9693.8 ->         0.0      -9694     OLDO  -0.07     floatedmValueMap_trackExtenderWithMTD_tmtd_RECO.
3084220.0 ->         0.0   -3084220     OLDO  -21.70     recoTrackExtras_trackExtenderWithMTD__RECO.
-------------------------------------------------------------
 14214626 ->     9554754   -4659871           -32.8     ALL BRANCHES

This looks in line with the expectation/description.

@fabiocos
Copy link
Contributor Author

@jpata yes, it confirms my test above (BTW, which tool is this compareProducts?)

@jpata
Copy link
Contributor

jpata commented Jun 24, 2021

@jpata yes, it confirms my test above (BTW, which tool is this compareProducts?)

it's this one, used by the bot when profiling: https://github.com/slava77/cms-reco-tools/blob/master/compareProducts.sh

@fabiocos
Copy link
Contributor Author

@jpata thanks, that might be useful as a standalone utility within CMSSW as well IMHO.

@perrotta
Copy link
Contributor

@jpata thanks, that might be useful as a standalone utility within CMSSW as well IMHO.

BTW, standalone usage is documented in https://twiki.cern.ch/twiki/bin/viewauth/CMS/RecoIntegration#Compare_output_products

@jpata
Copy link
Contributor

jpata commented Jun 25, 2021

+reconstruction

  • drops MTD content from AOD, while keeping it in RECO & FEVT
  • step3.root (AOD) size decreases by 1/3 in the phase2 workflow 23434.21
  • we see in the reco checks that trackExtenderWithMTD is dropped as expected

@jfernan2
Copy link
Contributor

+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)

@qliphy
Copy link
Contributor

qliphy commented Jun 25, 2021

+1

@cmsbuild cmsbuild merged commit 3d87b80 into cms-sw:master Jun 25, 2021
@fabiocos fabiocos deleted the fc-mtdevcont branch June 30, 2021 14:05
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.

None yet

7 participants