Break Reconstructions dependency on FastSimulation

[Dr15Jones]

Many reconstruction packages depend on the following two FastSimulation packages:

• FastSimulation/BaseParticlePropagator
• FastSimulation/CaloGeometryTools

I propose moving those packages (and any FastSimulation packages they depend upon) to a common Subsystem in order to properly reflect the dual usage of the packages.

This will also fix the dependency violations shown in the IB build summary pages.

[cmsbuild]

A new Issue was created by @Dr15Jones Chris Jones.

@davidlange6, @Dr15Jones, @smuzaffar, @fabiocos, @kpedro88 can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

[Dr15Jones]

assign reconstruction, fastsim

[cmsbuild]

New categories assigned: fastsim,reconstruction

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

[kpedro88]

Conceptually related to #22575

[Dr15Jones]

I propose that FastSimulation/BaseParticlePropagator and FastSimulation/CaloGeometryTools (as well as what they depend upon) be moved to CommonTools subsystem.

@slava77,@civanch,@ssekmen,@perrotta,@mdhildreth,@lveldere comments?

[davidlange6]

which refers to yet another package? Perhaps one needs to look at this at the class level..

…

On Feb 26, 2019, at 3:26 AM, Kevin Pedro ***@***.***> wrote: Conceptually related to #22575 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[Dr15Jones]

After an upcoming pull request where I refactor some of fastsim, the remaining uses of fastsim outside of FastSimulation subsystem are

GeneratorInterface/GenFilters/interface/BHFilter.h:#include "FastSimulation/BaseParticlePropagator/interface/BaseParticlePropagator.h"

L1Trigger/L1THGCal/plugins/ntuples/HGCalTriggerNtupleGen.cc:#include "FastSimulation/Event/interface/FSimEvent.h"
L1Trigger/L1THGCal/plugins/ntuples/HGCalTriggerNtupleGen.cc:#include "FastSimulation/CaloGeometryTools/interface/Transform3DPJ.h"

RecoEgamma/EgammaElectronProducers/plugins/LowPtGsfElectronSeedProducer.cc:#include "FastSimulation/BaseParticlePropagator/interface/BaseParticlePropagator.h"
RecoEgamma/EgammaElectronProducers/plugins/LowPtGsfElectronSeedProducer.cc:#include "FastSimulation/BaseParticlePropagator/interface/RawParticle.h"

RecoEgamma/EgammaTools/interface/EgammaPCAHelper.h:#include "FastSimulation/CaloGeometryTools/interface/Transform3DPJ.h"

RecoParticleFlow/PFSimProducer/plugins/ConvBremSeedProducer.cc:#include "FastSimulation/TrackerSetup/interface/TrackerInteractionGeometryRecord.h"
RecoParticleFlow/PFSimProducer/plugins/ConvBremSeedProducer.cc:#include "FastSimulation/ParticlePropagator/interface/MagneticFieldMapRecord.h"
RecoParticleFlow/PFSimProducer/plugins/ConvBremSeedProducer.cc:#include "FastSimulation/TrackerSetup/interface/TrackerInteractionGeometry.h"
RecoParticleFlow/PFSimProducer/plugins/ConvBremSeedProducer.cc:#include "FastSimulation/ParticlePropagator/interface/ParticlePropagator.h"
RecoParticleFlow/PFSimProducer/plugins/ConvBremSeedProducer.cc:#include "FastSimulation/TrajectoryManager/interface/InsideBoundsMeasurementEstimator.h"
RecoParticleFlow/PFSimProducer/plugins/ConvBremSeedProducer.cc:#include "FastSimulation/TrajectoryManager/interface/LocalMagneticField.h"
RecoParticleFlow/PFSimProducer/plugins/ConvBremSeedProducer.h:#include "FastSimulation/BaseParticlePropagator/interface/BaseParticlePropagator.h"
RecoParticleFlow/PFSimProducer/plugins/PFSimParticleProducer.cc:#include "FastSimulation/Event/interface/FSimEvent.h"
RecoParticleFlow/PFSimProducer/plugins/PFSimParticleProducer.cc:#include "FastSimulation/Event/interface/FSimTrack.h"
RecoParticleFlow/PFSimProducer/plugins/PFSimParticleProducer.cc:#include "FastSimulation/Event/interface/FSimVertex.h"
RecoParticleFlow/PFSimProducer/plugins/PFSimParticleProducer.h:#include "FastSimulation/Event/interface/FSimVertex.h"
RecoParticleFlow/PFSimProducer/plugins/TauHadronDecayFilter.cc:#include "FastSimulation/Event/interface/FSimEvent.h"
RecoParticleFlow/PFSimProducer/plugins/TauHadronDecayFilter.cc:#include "FastSimulation/Event/interface/FSimTrack.h"
RecoParticleFlow/PFSimProducer/plugins/TauHadronDecayFilter.cc:#include "FastSimulation/Event/interface/FSimVertex.h"

RecoParticleFlow/PFTracking/plugins/GoodSeedProducer.cc:#include "FastSimulation/BaseParticlePropagator/interface/BaseParticlePropagator.h"
RecoParticleFlow/PFTracking/src/PFTrackTransformer.cc:#include "FastSimulation/BaseParticlePropagator/interface/BaseParticlePropagator.h"

RecoTauTag/RecoTau/interface/AntiElectronIDMVA6.h:#include "FastSimulation/BaseParticlePropagator/interface/BaseParticlePropagator.h"
RecoTauTag/RecoTau/plugins/PFRecoTauChargedHadronFromTrackPlugin.cc:#include "FastSimulation/BaseParticlePropagator/interface/BaseParticlePropagator.h"
RecoTauTag/RecoTau/plugins/PFRecoTauChargedHadronFromTrackPlugin.cc:#include "FastSimulation/BaseParticlePropagator/interface/RawParticle.h"

If we ignore the uses in RecoParticleFlow/PFSimProducer, we are left with the following packages being used

• FastSimulation/BaseParticlePropagator by 4 packages
• FastSimulation/Event used only by L1Trigger/L1THGCal/plugins/ntuples/HGCalTriggerNtupleGen.cc
• FastSimulation/CaloGeometryTools/interface/Transform3DPJ.h used by L1Trigger/L1THGCal/plugins/ntuples/HGCalTriggerNtupleGen.cc and RecoEgamma/EgammaTools/interface/EgammaPCAHelper.h

FastSimulation/CaloGeometryTools/interface/Transform3DPJ.h is strange. It appears to be copied from ROOT and has no external dependencies. It definitely looks like it should not be in FastSimulation/CaloGeometryTools which has additional dependencies.

https://github.com/cms-sw/cmssw/blob/02d4198c0b6615287fd88e9a8ff650aea994412e/FastSimulation/CaloGeometryTools/interface/Transform3DPJ.h

[Dr15Jones]

The equivalent ROOT file appears to be
https://root.cern/root/html602/src/ROOT__Math__Transform3D.h.html

[Dr15Jones]

Actually, it looks like Transform3DPJ.h was copied from
https://github.com/root-project/root/blob/master/math/genvector/inc/Math/GenVector/Transform3D.h

and removed the template.

[Dr15Jones]

#26061 is the big refactoring I've done in preparation for moving the shared FastSimulation code to CommonTools.

[Dr15Jones]

#26070 removes one unnecessary dependency on FastSimulation//CaloGeometryTools/interface/Transform3DPJ.h from RECO code.

[Dr15Jones]

I just did a side-by-side comparison of Transform3DPJ.* and ROOT's Transform3D.* from 2008. All the optimizations in Transform3DPJ.* are already in Transform3D.*.

https://github.com/root-project/root/blob/a446719c104c411839f92ce960cf458842ae212f/math/genvector/inc/Math/GenVector/Transform3D.h
https://github.com/root-project/root/blob/a446719c104c411839f92ce960cf458842ae212f/math/genvector/src/Transform3D.cxx

Therefore we should remove Transform3DPJ.* from CMSSW and use ROOT's Transform3D.*.

[Dr15Jones]

The other use of Transform3DPJ outside of FastSim goes away with #26073.

[Dr15Jones]

#26073 removes the last dependency on Transform3DPJ outside of FastSimulation.

[fabiocos]

@davidlange6 @26187 indeed provides another (small) package, but it looks to me a clean solution. What alternative do you propose?

[Dr15Jones]

#26187 indeed provides another (small) package, but it looks to me a clean solution. What alternative do you propose?

It does not add another small package. It just moves the same small package from FastSimulation directory to the CommonTools directory.

[fabiocos]

@Dr15Jones you are right, the net balance is zero, it removes and adds back the same package, just elsewhere. As I said, it looks ok to me

[fabiocos]

@Dr15Jones #26187 has been now merged

[slava77]

If we ignore the uses in RecoParticleFlow/PFSimProducer

do I understand correctly that this dependence can stay and this issue can be closed?

[smuzaffar]

we can close this issue, only RecoParticleFlow/PFSimProducer now depend on FastSimulation.