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.

Sign up for GitHub

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

Jump to bottom

Standalone Delphes with EDM4HEP output #56

Merged
merged 50 commits into from Aug 11, 2020
Merged

Standalone Delphes with EDM4HEP output #56

merged 50 commits into from Aug 11, 2020

Conversation

vvolkl
Copy link
Collaborator

@vvolkl vvolkl commented May 18, 2020 •
edited by tmadlener

This PR disentangles the Delphes program from the ExRootAnalysis TreeWriter, and adds edm4hep output via the podio EventStore instead. The conversion is only a proof of principle now, and still WIP. The branches are created automatically from the TreeWriter configuration in the Delphes card.
I started out from DelphesROOT and cleared out some non-crucial code and replaced some raw pointers with smart pointers. Ultimately it would be good to make the input/output of the delphes standalone executables more modular (currently quite a bit of code is duplicated in 8 versions of Delphes with different inputs).
BEGINRELEASENOTES

  • Add a converter module for delphes that can output edm4hep files. The module is used in several standalone delphes executables that take different input formats (e.g. stdhep, pythia8, etc.) and standard delphes cards and produce edm4hep output root files. It can also be used in conjunction with gaudi. This is an early release to gather feedback and to iterate on that feedback. Currently not all of the input readers that come with standard delphes are implemented and also the information content of the edm4hep files is restricted to the most important information. More information can be added on demand. A brief introduction and some examples are included.

ENDRELEASENOTES

@tmadlener
Copy link
Contributor

I have rebased the pull request on the current master.
As discussed during todays meeting, I have removed the changes to the edm4hep::ReconstructedParticle (#69) and also the dependency on (podio/95).

Documentation of how the converter can be configured is still missing.

@vvolkl vvolkl changed the title [WIP] Standalone Delphes with EDM4HEP output Standalone Delphes with EDM4HEP output Jun 23, 2020
@tmadlener tmadlener mentioned this pull request Jul 3, 2020
Merged
@andresailer
Copy link
Collaborator

I think you need to rebase again.

vvolkl and others added 24 commits July 6, 2020 13:29
@vvolkl @tmadlener
add first version of delphes programs with edm4hep output
13ea31b
@vvolkl @tmadlener
save additional branches directly to ttree
132a5d7 
- more conversions
- add isolation vars
- add D0/DZ vars
@vvolkl @tmadlener
add pythia8, hepmc and stdhep input for delphes plugin
82c732c
@vvolkl @tmadlener
update podio prototypes with new podio version
3dcfb11
@vvolkl @tmadlener
Use ParticleIDs instead of standalone branches
c159130 
Co-authored-by: Thomas Madlener <thomas.madlener@desy.de>
@vvolkl @tmadlener
remove ROOTWriter Prototype
fb3141d 
With podio/95 they are no longer necessary
@tmadlener
Add tracks to output collections
46247cd
@tmadlener
Add usage messages for readers
88b02a9
@tmadlener
Fix small problem with interrupt not working
11e88cf
@tmadlener
Add some examples
bc435d7
@tmadlener
Store the tracks in the same collections as delphes
5ffcf50
@tmadlener
Add tracks to the ReconstructedParticles
4a65563
@tmadlener
Use right pointer type
b113b56
@tmadlener
Use delphes helpers to get mc <-> reco relations
ef0fe97
@tmadlener
Fix setting of reconstructed masses
d002e20 
Needs to be done after the momentum has been set in order for the
calculation of the energy from the passed masses is done consistently
@tmadlener
Add tracks to jets via delphes helper tools
4a64263 
TODO: Check differences between delphes and edm4hep output
@tmadlener
Add towers as clusters
cc73b04 
TODO: currently the edm4hep jets have much less associated clusters than
the delphes clusters
@tmadlener
Start to work on a cleaner implementation
6c4c71e
@tmadlener
Add gen particles, tracks and towers
f40df44
@tmadlener
Add jets
6620591
@tmadlener
Add muons, electrons and photons
f243ebb 
Had to introduce a new class of RecoParticleRef that allows us to store
collections of particle references which only point into the global
reconstructed particle collection. This is necessary to not duplicate
reconstructed particles when splitting them into subsets.
@tmadlener
Properly handle gen particles of jet constituents
a403e58
@tmadlener
Update examples
bd6bf30
@tmadlener
Avoid double counting of the same gen particle
5d1a66d
tmadlener and others added 10 commits July 6, 2020 13:29
@tmadlener
Add MissingET and ScalarHT
3950d8b
@tmadlener
Add config class and read it from .tcl file
a0b9552
@tmadlener
Use Delphes class that is shipped with delphes
a8ab8a8 
A fork of this class was used up until now. The main goal was to be able
to use unchanged delphes cards. However, since the EDM4HepOutput module
has to be defined in any case, another change to the delphes card should
no longer be a real problem.
@tmadlener
Remove no longer used file
c45f0a2
@tmadlener
Use const_cast to not be reliant on podio patch
f1e2b28 
As discussed during the meeting, a final decision on how to best handle
this needs some more discussion. For now using a const_cast here, to
showt that some more work is necessary.
@tmadlener
Cleanup and removal of unused code
6fab6eb
@tmadlener
Add test for RecoParticleRef
28d8517
@tmadlener
Update documentation
357c231
@tmadlener
Remove filesystem to avoid linker problems
a63c2a3 
gcc8 (used in e.g. CI) would require separately linking in stdc++fs as
it is only merged into the stl library with gcc9.
@vvolkl @tmadlener
replace const cast with registerCollection (#70)
5e8d97f
andresailer
andresailer reviewed Jul 13, 2020
View reviewed changes
plugins/delphes/CMakeLists.txt Outdated Show resolved Hide resolved
plugins/delphes/CMakeLists.txt Outdated Show resolved Hide resolved
plugins/delphes/CMakeLists.txt Outdated Show resolved Hide resolved
plugins/delphes/CMakeLists.txt Outdated Show resolved Hide resolved
plugins/delphes/CMakeLists.txt Outdated Show resolved Hide resolved
plugins/delphes/DelphesPythia8Reader.h Outdated Show resolved Hide resolved
plugins/delphes/DelphesRootReader.h Outdated Show resolved Hide resolved
plugins/delphes/DelphesRootReader.h Outdated Show resolved Hide resolved
plugins/delphes/examples/common_helpers.h Show resolved Hide resolved
plugins/delphes/examples/common_helpers.h Outdated Show resolved Hide resolved
@andresailer
Copy link
Collaborator

TODO: Change to allow use of un-changed delphes cards

@gaede
Copy link
Collaborator

gaede commented Aug 4, 2020

@vvolkl and @tmadlener: please add a few lines to the Release notes

tmadlener and others added 3 commits August 5, 2020 16:02
@tmadlener
Copy link
Contributor

Hi @vvolkl,
Looking at your last commit I see that you introduce one global ParticleIDCollection. Would it make sense to have one collection per output collection? I am mainly thinking about the tooling support, like some utility functionality that facilitates accessing the information stored in the ParticleIDs. With different collection we could put, e.g. the meaning of the different indices, into the metadata of each collection. I am not entirely sure whether we need separate collections for that or whether we could still do that with one collection and slightly more elaborate metadata.

For now I think this can go in as it is, but we may need to revisit this in a future iteration. The question is mainly: Where do we want to do the bookkeeping.

This also needs to be rebased onto master again if we do not want to have a merge commit in the history.

@vvolkl
Copy link
Collaborator Author

vvolkl commented Aug 11, 2020

Hi Thomas,
Yes, I thought about this, but at the moment I don't see the need, and with one global collection it is simpler. But we can indeed revisit this in the future.

For me the rebase and merge button still works - I'll do one more round of testing, then I'll merge.

@vvolkl vvolkl merged commit b8ec3dc into master Aug 11, 2020
@vvolkl vvolkl deleted the delphes branch August 11, 2020 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andresailer andresailer andresailer left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@vvolkl @tmadlener @andresailer @gaede