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

Preferred way to break DataFormats/TrackReco and DataFormats/TrackCandidate circle? #31301

Closed
davidlange6 opened this issue Aug 31, 2020 · 12 comments

Comments

@davidlange6
Copy link
Contributor

There is a ~trivially fixed circular dependency between DataFormats/TrackReco and DataFormats/TrackCandidate. Which package is meant to depend on the other. I can move things accordingly (either some dictionaries that use classes from both packages or one header)

@cmsbuild
Copy link
Contributor

A new Issue was created by @davidlange6 David Lange.

@Dr15Jones, @dpiparo, @silviodonato, @smuzaffar, @makortel, @qliphy can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

@davidlange6
Copy link
Contributor Author

assign reconstruction

@cmsbuild
Copy link
Contributor

New categories assigned: reconstruction

@slava77,@perrotta,@jpata you have been requested to review this Pull request/Issue and eventually sign? Thanks

@slava77
Copy link
Contributor

slava77 commented Aug 31, 2020

@davidlange6
I'm missing details or being confused by the phrasing (perhaps as trivial as a question mark in the "Which package is meant to depend on the other.")

@slava77
Copy link
Contributor

slava77 commented Aug 31, 2020

after looking a bit more, I see that the direct dependence of TrackReco on TrackCandidate is only in the dictionaries definition in classes.h used for an aggregate <TrackCandidate, Track> definition.

The dependence of TrackCandidate is apparently via
DataFormats/TrackCandidate/interface/TrackCandidate.h:
#include "DataFormats/TrackReco/interface/TrajectoryStopReasons.h"

@davidlange6
please clarify if this is what you meant with the issue description.

@davidlange6
Copy link
Contributor Author

davidlange6 commented Aug 31, 2020 via email

@slava77
Copy link
Contributor

slava77 commented Aug 31, 2020

@mtosi @vmariani

@rovere
it looks like you are the original developer and perhaps recall the history.

Naively, it looks to me that DataFormats/TrackReco/interface/TrajectoryStopReasons.h can be moved to DataFormats/TrackCandidate/interface/

@davidlange6
Copy link
Contributor Author

davidlange6 commented Aug 31, 2020 via email

@rovere
Copy link
Contributor

rovere commented Sep 1, 2020

@mtosi @vmariani

@rovere
it looks like you are the original developer and perhaps recall the history.

Naively, it looks to me that DataFormats/TrackReco/interface/TrajectoryStopReasons.h can be moved to DataFormats/TrackCandidate/interface/

From a quick look, I tend to agree on that. That would also imply to move the corresponding *cc.

@slava77
Copy link
Contributor

slava77 commented Sep 1, 2020

FWIW, TrajectoryStopReasons is actually a track candidate stop reason.
So, the relocation seems reasonable.

@slava77
Copy link
Contributor

slava77 commented Sep 5, 2020

+1

fixed in #31327 (merged Sept 3)
the dependency cycle is gone, according to the ignominy report in https://cmssdt.cern.ch/SDT/cgi-bin/newQA.py?arch=slc7_amd64_gcc820&release=CMSSW_11_2_X_2020-09-04-2300

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 5, 2020

This issue is fully signed and ready to be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants