Pre release cleanup #31
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Delete the less performant tiled algorithms
Using LV inputs gives a standard interface to the reconstruction algorithms Implement HepMC reader that can return LVs Adapt N2Plain algorithm to digest these inputs (there is a slight performance regression, 780->833 us on my machine) Clean up all of the "dev' code from the N2Plain algorithm
These are much more standard and supported objects from the LorentzVectorHEP package This drops use of the internal "Particle.jl" struct. Do not export PseudoJet functions. Some standard reformatting applied
This is used in N2Plain Adapt tests to allow for algorithms returning phi in the [-\pi, \pi] range
New version supports rapidity() method, as well as pt2() and more consistent handling of mass()
This was mistakenly masked in .gitignore
Do not export "internal" pieces from JetReconstruction Add "import" of relevant methods from LorentzVectorHEP for use in the algorithms
Add an outer "wrapper" that writes the internal EDM objects and allow for a direct call when the internal EDM is already constructed (this is fairer for timings) Refactor many of the variable names to get rid of the underscore prefixes and to make it clearer when variables are arrays, rather than single values
The conversion from LorentzVectorHEP to PseudoJet is rather expensive and should be factored out of the benchmark for a fair comparison of the actual algorithm
We want to have the full suite of relevant methods available in our namespace
Change rap() to rapidity() for consistency
This makes the struct more generic and avoids directly accessing member variables
Changing back to PseudoJets, make the generic method for N2Tiled type agnostic (anything that supports energy(), px(), py(), pz() is fine)
As N2Plain is written, it's completely happy to use PseudoJets directly
Anything non-standard will require recombine(::PseudoJet, ::PseudoJet) to be defined.
To make the interface the same as N2Tiled support directly filtering on the final jets' ptmin values (which actually speeds up the code, slightly!)
Even from N2Plain we return PseudoJet types (but avoid construction of new objects if possible)
Check that reconstruction from LorentzVector gives the same as PseudoJet
Write a proper README file that describes how to use the package (real Julia documentation to come, but it's a start) Rename main algorithms to be more consistent, viz. sequential_jet_reconstruct and tiled_jet_reconstruct Also remove all of the pre-cooked versions with specific values of p (like cambridge_aachen_algo) as these add little value and would need to exist for all algorithms. Rename the chep.jl script to jetreco.jl and put it into an examples directory
Moelf
reviewed
Oct 25, 2023
Co-authored-by: Jerry Ling <[email protected]>
|
I'm not familiar with the content and FastJet APIs so can't say much :P But seems to be a huge step towards being a proper library package in terms of organization |
Thanks for spotting the non const issue!
Thank you! I hope no obvious defects or daft things done. |
Moelf
approved these changes
Oct 25, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a significant clean-up, taking us close to an actual release of this package.
The two underperforming algorithms (combining tiled and SoA) have been removed.
The two remaining algorithms have had their main APIs brought into line with one another:
plain_jet_reconstruct(particles::Vector{T}; p = -1, R = 1.0, recombine = +, ptmin = 0.0)andtiled_jet_reconstruct(particles::Vector{T}; p = -1, R = 1.0, recombine = +, ptmin = 0.0).For the input particle type,
JetReconstruction.PseudoJetandLorentzVectorHEPtypes are supported, but, in general, any type can be used that supports appropriate 4-vector methods (pt2(),phi(),rapidity(),px(),py(),pz(),energy()).The return type of both algorithms is
JetReconstruction.PseudoJetas this is the most complete and efficient type - translation toLorentzVectorHEPis trivial, but always doing it is rather expensive (particularly as cylindrical vectors fromLorentzVectorHEPstore pseudorapidity and jet reconstruction uses rapidity).The old style
Particle.jltype is removed.Significant amounts of renaming of files, methods and variables have been done to help clarity.
The
READMEis reworked to be more useful.There are still some defects that need to be addressed (see open issues), but a lot is now fixed.
Closes #5 (LorentzVectorHEP is supported, but is problematic to use internally)
Closes #15 (I think we got really pretty close to optimal now)
Closes #23 (API is well defined now, as is use of alternative input types)
Closes #25 (MIT everywhere)