Update Tiled reconstruction to use `nothing` as start/stop of list
Currently the TiledJet stuct is used as a manually managed linked list for tiled jet reconstruction.
Changing this to a Julia DataStuctures.jl MutableLinkedList would be quite desirable, and would also help with #97, which got stuck because of problems in parameterising the noTiledJet that was used to indicate the start/end points of the list and would no longer be needed.
https://github.com/JuliaCollections/DataStructures.jl/pull/873
looks a bit shaky
I'd still like to try this out - it would simplify the code quite a bit. I also want to see the impact on performance.
As MutableLinkedList looks a bit flaky, thus a risky option, we could improve significantly by changing the TiledJet so that the next and previous values indicate the ends of the linked list using nothing.
The the parameters would be:
next::Union{TiledJet, Nothing}
prev:: Union{TiledJet, Nothing}
This should be totally optimisable by the compiler these days and will unblock the parameterisation issue #91, where the PR #97 got stuck on noTiledJet.
I'd really like to address this bit of technical debt (fixed Float64) for v1.0.0.
Well, I just tried noTiledJet -> nothing and it loses about 15% performance, so this is not a change we could make lightly.
~/.julia/dev/JetReconstruction/examples/ [tiledjet-nothing*] julia --project instrumented-jetreco.jl --algorithm=AntiKt -R 0.4 ../test/data/events.pp13TeV.hepmc3.zst -S N2Tiled -m 32
Processed 100 events 32 times
Average time per event 280.8702334375 ± 11.376835488736306 μs
Lowest time per event 269.03458 μs
change from
~/.julia/dev/JetReconstruction/examples/ [main*] julia --project instrumented-jetreco.jl --algorithm=AntiKt -R 0.4 ../test/data/events.pp13TeV.hepmc3.zst -S N2Tiled -m 32
Processed 100 events 32 times
Average time per event 239.37884062499998 ± 9.407835991083875 μs
Lowest time per event 228.98333 μs
union splitting not kicking in 🤔 ?
I think the problem is the there's also an Array of tiles which contains the first tile in any of the linked lists (it's the entry point). As it's possible for any tile to contain no active jets this also has to be :: Union{TiledJet, Nothing}. As there's a loop over these tiles this can't union split, as it's an internal decision point.
I have some ideas, e.g., using a self-refrerence to indicate the start/end of the list (but an empty tile also needs to be signalled, maybe with a TiledJet with id=-1).
For all it is inflexible, the current implementation is extremely efficient. Chapeau! 🎩