Skip to content

Add E-scheme preprocessor fixing cluster history indices#211

Merged
graeme-a-stewart merged 7 commits intoJuliaHEP:mainfrom
m-fila:cluster_hsitory_indices
Dec 8, 2025
Merged

Add E-scheme preprocessor fixing cluster history indices#211
graeme-a-stewart merged 7 commits intoJuliaHEP:mainfrom
m-fila:cluster_hsitory_indices

Conversation

@m-fila
Copy link
Member

@m-fila m-fila commented Nov 16, 2025

The ptscheme and pt2scheme preprocessors already assign correct history indices to jet objects. The default escheme does not currently have its own preprocessor: it either copies the full vector of jets when they are already PseudoJet/EEJet (assuming their cluster indices are correct), or it converts to PseudoJet/EEJet and assigns new history indices during that conversion.

As described in #200, relying on the input jets for escheme to already have correct cluster indices can be easy to overlook. Instead of performing checks and index fix-ups as suggested there, this proposal introduces a dedicated preprocessor for escheme that always copies jets and assigns correct history indices. This would become the default behavior for e-scheme (other scheme preprocessors already include fixing indices). Users who know their input jets already have valid history indices can opt out by specifying nothing as the preprocessor to avoid unnecessary work.

I also noticed that two forms of preprocessor functions exist: one that accepts an output type and performs conversion, and one that does not. The docs describes user-provided preprocessors as the non-converting type, but the implementation only uses the converting form. I fix the docs and remove the unused methods.

Finally, this PR includes fixes for history-index in SoftKiller (#191), as it is related and there has been no recent activity on that issue.

Closes #200 #191

@m-fila m-fila changed the title Cluster hsitory indices Add E-scheme preprocessor fixing cluster history indices Nov 16, 2025
@codecov
Copy link

codecov bot commented Nov 16, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.34%. Comparing base (2fc91d2) to head (1701918).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #211      +/-   ##
==========================================
+ Coverage   80.22%   80.34%   +0.11%     
==========================================
  Files          21       21              
  Lines        1315     1338      +23     
==========================================
+ Hits         1055     1075      +20     
- Misses        260      263       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@m-fila
Copy link
Member Author

m-fila commented Nov 16, 2025

Performance wise it seems calling the preprocess for fixing indices isn't more expensive than nothing preprocess just coping input jets without fixups:

  • preprocess_escheme
    pp 14TeV Tiled
    Processed 100 events 16 times
    Average time per event 177.05440187500005 ± 18.650765441555805 μs
    Lowest time per event 161.72569 μs
    pp 14 TeV Plain
    Processed 100 events 16 times
    Average time per event 504.513385625 ± 18.07148126798454 μs
    Lowest time per event 486.39413 μs
    ee H Durham
    Processed 100 events 16 times
    Average time per event 32.36774750000001 ± 3.0916310772200886 μs
    Lowest time per event 29.67452 μs
    
  • nothing
    pp 14TeV Tiled
    Processed 100 events 16 times
    Average time per event 176.36583312499997 ± 17.66222657424268 μs
    Lowest time per event 162.19935999999998 μs
    pp 14 TeV Plain
    Processed 100 events 16 times
    Average time per event 501.3887362499999 ± 16.77077525920156 μs
    Lowest time per event 485.47028 μs
    ee H Durham
    Processed 100 events 16 times
    Average time per event 35.764484375 ± 4.081166524662498 μs
    Lowest time per event 29.568640000000002 μs
    

@m-fila m-fila linked an issue Nov 16, 2025 that may be closed by this pull request
@graeme-a-stewart graeme-a-stewart self-requested a review December 8, 2025 13:47
m-fila and others added 6 commits December 8, 2025 16:22
Some clarity improvements to the recombination documentation.

Add a constructor for EEJet that takes (pt, y, phi, m) meaning that
PtScheme and Pt2Scheme now work for EEJet (they are not really favoured,
but should not crash!).

Add a --recombine option to jetreco.jl as for completeness.
Copy link
Member

@graeme-a-stewart graeme-a-stewart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great patch @m-fila - this is indeed a better way of tacking the consistency for the history index.

I tweaked the docs a bit, and also fixed some issues with using recombination that needs $(p_T, y, \phi)$ with EEJet (which was broken...).

@graeme-a-stewart
Copy link
Member

I also measured the performance here and couldn't see any measurable performance downsides with setting the history index as a default in EScheme, so this is good to go.

@graeme-a-stewart graeme-a-stewart merged commit 1d3cae2 into JuliaHEP:main Dec 8, 2025
14 checks passed
@m-fila m-fila deleted the cluster_hsitory_indices branch December 8, 2025 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Automatically fix-up jet cluster_hist_seq when (re)running reconstruction SoftKiller should return PseudoJets with the correct cluster_hist_index

2 participants