-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Reduce dependency on eigen #31735
Comments
A new Issue was created by @Dr15Jones Chris Jones. @Dr15Jones, @dpiparo, @silviodonato, @smuzaffar, @makortel, @qliphy can you please review it and eventually sign/assign? Thanks. cms-bot commands are listed here |
assign reconstruction, upgrade |
#31722 adds a dependence |
@VinInn <https://github.com/VinInn> @fwyzard <https://github.com/fwyzard>
can you look at choleskyInversion?
No, but feel free to make a PR, and somebody will review it.
… |
Historically Math is in DataFormat. Was not my decision as was not my decision to swallow SMatrix in Root. |
@davidlange6 FYI |
Could someone explain why we need to reduce the dependency from Eigen? |
IIUC, it's for DataFormats. |
No data-type eigen-specific is used to persist anything, IIRC. |
Same applies to Math Geometry* etc. |
The request is not specific to Eigen but is true in general (you can see other recent pull requests I've done to reduce DataFormats dependencies on externals). Beyond the good practice in large scale software design to minimize dependencies in order to improve build times and eas maintenance, classes in DataFormats have some additional reasons to minimize external dependencies
|
@Dr15Jones |
I think to first order that is correct. On a package basis, I think it just increases the time and memory needed to compile the code. |
FWLite only depends on eigen because of the tenous use of that package by DataFormats/HGCalReco and DataFormats/Math.
DataFormats/HGCalReco/interface/Trackster.h uses eigen only in a member function that takes eigen classes and converts them into std::array's. This dependency could be removed either by converting that function to be templated (since it is all inlined anyway) or by using a stand alone function to do the conversion. The function in question,
Trackster::fillPCAVariables
is only ever called from RecoHGCal/TICL/plugins/TrackstersPCA.cc so the conversion could be done there.DataFormats/Math/interface/choleskyInversion.h is the only use of eigen in DataFormats/Math and the header is not used anywhere in CMSSW except in the unit test for the function. This file could either be removed entirely from CMSSW or moved to a non DataFormats package.
The text was updated successfully, but these errors were encountered: