Revisit usage of HitPattern in DataFormats/Scouting #37149

mmasciov · 2022-03-05T19:30:52Z

PR description:

This PR is meant to address issue #32219 and supersedes PR #35685.
It is introducing a new Run3ScoutingHitPatternPOD class in DataFormats/Scouting, to be used for Run-3 scouting (in Run3ScoutingMuon). The class is a POD-like version of reco::HitPattern.
A function is defined in reco::HitPattern to fill Run3ScoutingHitPatternPOD, together with a reco::HitPattern constructor from Run3ScoutingHitPatternPOD.

FYI @dsperka, @makortel

cmsbuild · 2022-03-05T19:39:04Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37149/28695

This PR adds an extra 36KB to repository

cmsbuild · 2022-03-05T19:39:30Z

A new Pull Request was created by @mmasciov (Mario Masciovecchio) for master.

It involves the following packages:

DataFormats/Scouting (core)
DataFormats/TrackReco (reconstruction)
HLTrigger/Muon (hlt)

@smuzaffar, @Dr15Jones, @makortel, @clacaputo, @cmsbuild, @missirol, @Martin-Grunewald, @slava77, @jpata can you please review it and eventually sign? Thanks.
@sscruz, @Fedespring, @missirol, @silviodonato, @trocino, @abbiendi, @JanFSchulte, @jhgoh, @VinInn, @Martin-Grunewald, @calderona, @HuguesBrun, @rovere, @CeliaFernandez, @gpetruc, @mmusich, @mtosi, @cericeci this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

DataFormats/Scouting/interface/Run3ScoutingHitPatternPOD.h

DataFormats/Scouting/src/classes_def.xml

DataFormats/TrackReco/src/HitPattern.cc

missirol · 2022-03-05T20:32:20Z

please test

cmsbuild · 2022-03-05T22:41:49Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37149/28697

This PR adds an extra 36KB to repository

cmsbuild · 2022-03-05T22:42:06Z

Pull request #37149 was updated. @smuzaffar, @Dr15Jones, @makortel, @clacaputo, @cmsbuild, @missirol, @Martin-Grunewald, @slava77, @jpata can you please check and sign again.

missirol · 2022-03-05T22:47:06Z

please test

missirol · 2022-03-08T10:33:52Z

+hlt

clacaputo · 2022-03-08T11:09:13Z

+reconstruction

technical
no reco changes expected or observed

makortel · 2022-03-08T14:28:46Z

DataFormats/Scouting/interface/Run3ScoutingMuon.h

-        trk_hitPattern_(trk_hitPattern),
-        vtxIndx_(std::move(vtxIndx)) {}
+        vtxIndx_(std::move(vtxIndx)),
+        trk_hitPattern_(trk_hitPattern) {}


Given the std::vector member this would avoid one copy of it with std::move

Suggested change

trk_hitPattern_(trk_hitPattern) {}

trk_hitPattern_(std::move(trk_hitPattern)) {}

(sorry for noticing only now, I'm fine with a follow-up PR for that given the time pressure)

I can update this PR right away, if it works too

@qliphy @perrotta Do you have preferences (or shall we wait for ORP)?

Let have this PR merged now.
Please @mmasciov prepare a follow-up one-line PR with the proposed fix as soon as you can (even right after this one is merged on top of the master HEAD).

@perrotta, @makortel: done in PR #37176

makortel · 2022-03-08T14:28:57Z

+core

cmsbuild · 2022-03-08T14:29:17Z

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

mmasciov · 2022-03-08T14:58:53Z

@silviodonato, @dsperka, @missirol: as this is now fully signed, should I also submit a backport PR to 122X?

missirol · 2022-03-08T15:07:37Z

should I also submit a backport PR to 122X?

I'd say no; I don't think we want to change the DataFormats in 12_2_X, unless I'm missing something.

perrotta · 2022-03-08T15:08:42Z

+1

perrotta · 2022-03-09T07:00:26Z

@mmasciov after the merging of this PR in CMSSW_12_3_X_2022-03-08-2300 there are quite a lot of building errors in FWLite, see https://cmssdt.cern.ch/SDT/cgi-bin/showBuildLogs.py/fwlite/slc7_amd64_gcc10/www/tue/12.3-tue-23/CMSSW_12_3_X_2022-03-08-2300

/data/cmsbld/jenkins/workspace/build-fwlite-ib/cms/tmp/BUILDROOT/7d473ee20b53f0097cac9fcc6d90a6a3/opt/cmssw/slc7_amd64_gcc10/cms/fwlite/CMSSW_12_3_X_2022-03-08-2300_FWLITE/src/DataFormats/TrackReco/interface/HitPattern.h:127:10: fatal error: DataFormats/Scouting/interface/Run3ScoutingHitPatternPOD.h: No such file or directory
  127 | #include "DataFormats/Scouting/interface/Run3ScoutingHitPatternPOD.h"
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.

Could you please have a look and provide a fix at your earliest?

Martin-Grunewald · 2022-03-09T07:44:31Z

Usually it isn't needed, but perhaps FWLite requires the DataFormats/Scouting BuildFile to include <use name="DataFormats/Scouting"/> ?

missirol · 2022-03-09T09:49:51Z

I wonder if DataFormats/Scouting should be added to
https://github.com/cms-sw/cmsdist/blob/IB/CMSSW_12_3_X/master/fwlite_build_set.file

Kindly asking feedback to @smuzaffar .

smuzaffar · 2022-03-09T09:57:40Z

yes DataFormats/Scouting should be added to https://github.com/cms-sw/cmsdist/blob/IB/CMSSW_12_3_X/master/fwlite_build_set.file . This package only depend on DataFormats/Common ( which is already part of fwlite release) so there is no harm added DataFormats/Scouting to fwlite. I will update it for 11h00 Ib

smuzaffar · 2022-03-09T09:58:29Z

done cms-sw/cmsdist@94774dd

mmasciov · 2022-03-09T13:19:49Z

Thanks! Just for completeness, this appears to be fixed in the 11h00 IB: https://cmssdt.cern.ch/SDT/cgi-bin/showBuildLogs.py/fwlite/slc7_amd64_gcc10/www/wed/12.3-wed-11/CMSSW_12_3_X_2022-03-09-1100

Revisit usage of HitPattern in DataFormats/Scouting

bd4527d

cmsbuild added this to the CMSSW_12_3_X milestone Mar 5, 2022

cmsbuild added code-checks-pending core-pending hlt-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Mar 5, 2022

mmasciov mentioned this pull request Mar 5, 2022

Using new Run3ScoutingHitPattern class for hitPattern in Run-3 scouting #35685

Closed

cmsbuild added code-checks-approved and removed code-checks-pending labels Mar 5, 2022

missirol reviewed Mar 5, 2022

View reviewed changes

cmsbuild added tests-started and removed tests-pending labels Mar 5, 2022

mmasciov added 2 commits March 5, 2022 23:20

Include cstdint to fix header consistency

c7a0493

First rounnd of comments from PR review

7e8a182

cmsbuild added code-checks-pending tests-pending and removed tests-started code-checks-approved labels Mar 5, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Mar 5, 2022

cmsbuild removed the tests-pending label Mar 5, 2022

cmsbuild added tests-approved and removed tests-started labels Mar 8, 2022

cmsbuild added hlt-approved and removed hlt-pending labels Mar 8, 2022

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Mar 8, 2022

makortel reviewed Mar 8, 2022

View reviewed changes

cmsbuild added core-approved fully-signed and removed core-pending pending-signatures labels Mar 8, 2022

cmsbuild added orp-approved and removed orp-pending labels Mar 8, 2022

cmsbuild merged commit 692e417 into cms-sw:master Mar 8, 2022

mmasciov mentioned this pull request Mar 8, 2022

Add std::move to std::vector member of Run3ScoutingMuon #37176

Merged

missirol mentioned this pull request Mar 8, 2022

Revisit use of HitPattern in DataFormats/Scouting #32219

Closed

cmsbuild mentioned this pull request Mar 9, 2022

[G4 VECGEOM] Geant4 update to 11.0.1 patch cms-sw/cmsdist#7678

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Revisit usage of HitPattern in DataFormats/Scouting #37149

Revisit usage of HitPattern in DataFormats/Scouting #37149

mmasciov commented Mar 5, 2022

cmsbuild commented Mar 5, 2022

cmsbuild commented Mar 5, 2022

missirol commented Mar 5, 2022

cmsbuild commented Mar 5, 2022

cmsbuild commented Mar 5, 2022

missirol commented Mar 5, 2022

missirol commented Mar 8, 2022

clacaputo commented Mar 8, 2022

makortel Mar 8, 2022

mmasciov Mar 8, 2022

makortel Mar 8, 2022

perrotta Mar 8, 2022

mmasciov Mar 8, 2022

makortel commented Mar 8, 2022

cmsbuild commented Mar 8, 2022

mmasciov commented Mar 8, 2022

missirol commented Mar 8, 2022

perrotta commented Mar 8, 2022

perrotta commented Mar 9, 2022

Martin-Grunewald commented Mar 9, 2022

missirol commented Mar 9, 2022

smuzaffar commented Mar 9, 2022

smuzaffar commented Mar 9, 2022

mmasciov commented Mar 9, 2022

	trk_hitPattern_(trk_hitPattern) {}
	trk_hitPattern_(std::move(trk_hitPattern)) {}

Revisit usage of HitPattern in DataFormats/Scouting #37149

Revisit usage of HitPattern in DataFormats/Scouting #37149

Conversation

mmasciov commented Mar 5, 2022

PR description:

cmsbuild commented Mar 5, 2022

cmsbuild commented Mar 5, 2022

missirol commented Mar 5, 2022

cmsbuild commented Mar 5, 2022

cmsbuild commented Mar 5, 2022

missirol commented Mar 5, 2022

missirol commented Mar 8, 2022

clacaputo commented Mar 8, 2022

makortel Mar 8, 2022

Choose a reason for hiding this comment

mmasciov Mar 8, 2022

Choose a reason for hiding this comment

makortel Mar 8, 2022

Choose a reason for hiding this comment

perrotta Mar 8, 2022

Choose a reason for hiding this comment

mmasciov Mar 8, 2022

Choose a reason for hiding this comment

makortel commented Mar 8, 2022

cmsbuild commented Mar 8, 2022

mmasciov commented Mar 8, 2022

missirol commented Mar 8, 2022

perrotta commented Mar 8, 2022

perrotta commented Mar 9, 2022

Martin-Grunewald commented Mar 9, 2022

missirol commented Mar 9, 2022

smuzaffar commented Mar 9, 2022

smuzaffar commented Mar 9, 2022

mmasciov commented Mar 9, 2022