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

add labeling for tracking #1916

Merged
merged 4 commits into from Jan 18, 2023
Merged

add labeling for tracking #1916

merged 4 commits into from Jan 18, 2023

Conversation

slava77
Copy link
Contributor

@slava77 slava77 commented Jan 18, 2023

add packages and subsystems to be labeled for tracking

  • "CUDADataFormats/Track",
  • "DataFormats/Track",
  • "RecoPixelVertexing",
  • "RecoTracker",
  • "RecoVertex",
  • "TrackPropagation",
    • will also match to TrackPropagation/SteppingHelixPropagator, which is mostly muon reco, but it's probably still OK for tracking
  • "TrackingTools"
    • will also match TrackingTools/TrackAssociator, which is not tracking, but the alternative is to write a much longer list. So, we could probably unassign

@mmusich
do you have any comments?

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @slava77 (Slava Krutelyov) for branch master.

@cmsbuild, @smuzaffar, @aandvalenzuela, @iarspider can you please review it and eventually sign? Thanks.
@perrotta, @dpiparo, @rappoccio you are the release manager for this.
cms-bot commands are listed here

@cmsbuild
Copy link
Contributor

Pull request #1916 was updated.

@mmusich
Copy link
Contributor

mmusich commented Jan 18, 2023

does it mean they will get labelled automatically?

"RecoTracker",
"RecoVertex",
"TrackPropagation",
"TrackingTools"
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if we need also RecoPixelVZero and RecoVZero (do not exist anymore since a long time).
Do we need to flag also non reconstruction packages?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we need to flag also non reconstruction packages?

good point, here is more that matches

  • CUDADataFormats/Vertex
  • DataFormats/VertexReco
  • DQM/Tracking
  • Validation/RecoPixelVertexing
  • Validation/RecoTrack
  • Validation/RecoVertex
  • Validation/Tracking
  • FastSimulation/Tracking
  • Geometry/GlobalTracking

what else?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if we need also RecoPixelVZero and RecoVZero (do not exist anymore since a long time).

these were apparently removed in 9_2_X ( cms-sw/cmssw#18623) 10_0_X (cms-sw/cmssw#21456), respectively
But I guess I can add these as well

Copy link
Contributor

@mmusich mmusich Jan 18, 2023

Choose a reason for hiding this comment

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

I would add:

  • CondFormats/BeamSpotObjects
  • AnalysisDataFormats/TrackInfo
  • CondCore/BeamSpotPlugins
  • CondFormats/BeamSpotObjects
  • CondTools/BeamSpot
  • DQM/BeamMonitor
  • Validation/TrackingMCTruth (already covered above)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would add:

now in f2c36cc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually in 9d8c766

@cmsbuild
Copy link
Contributor

Pull request #1916 was updated.

@slava77
Copy link
Contributor Author

slava77 commented Jan 18, 2023

does it mean they will get labelled automatically?

yes

@cmsbuild
Copy link
Contributor

Pull request #1916 was updated.

@mmusich
Copy link
Contributor

mmusich commented Jan 18, 2023

looks good to me!

@mmusich
Copy link
Contributor

mmusich commented Jan 18, 2023

there is tracking related code in CommonTools/RecoAlgos and possibly also scattered in other packages. I guess we can still flag by hand when needed.

@slava77
Copy link
Contributor Author

slava77 commented Jan 18, 2023

there is tracking related code in CommonTools/RecoAlgos and possibly also scattered in other packages. I guess we can still flag by hand when needed.

right, I don't think there is a way to tag by individual files.

@smuzaffar
Copy link
Contributor

files information is available and we can use that for these labels.

@mmusich
Copy link
Contributor

mmusich commented Jan 18, 2023

files information is available and we can use that for these labels.

doing a survey down to the file level now would be extremely tedious. I would suggest to continue incorporating files in the list once concrete requests materialize. What do you think?

@slava77
Copy link
Contributor Author

slava77 commented Jan 18, 2023

files information is available and we can use that for these labels.

would this work already, or does it need some updates in the bot?

@smuzaffar
Copy link
Contributor

smuzaffar commented Jan 18, 2023

files information is available and we can use that for these labels.

would this work already, or does it need some updates in the bot?

No, it needs changes in bot code.

@smuzaffar
Copy link
Contributor

@slava77 , should I update bot to do the matching against the changed file ?

@slava77
Copy link
Contributor Author

slava77 commented Jan 18, 2023

@slava77 , should I update bot to do the matching against the changed file ?

I think that we can merge this PR first and then benefit from the file-level selection when it is available.

@smuzaffar smuzaffar merged commit 7da372d into cms-sw:master Jan 18, 2023
@smuzaffar
Copy link
Contributor

this has been merged and bot properly added the label for cms-sw/cmssw#30373

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

4 participants