Skip to content

Add support for RapidSim names#756

Open
admorris wants to merge 1 commit into
scikit-hep:mainfrom
admorris:rapidsim_names
Open

Add support for RapidSim names#756
admorris wants to merge 1 commit into
scikit-hep:mainfrom
admorris:rapidsim_names

Conversation

@admorris
Copy link
Copy Markdown
Contributor

I added support for the RapidSim particle-naming convention (oh how I wish they just used EvtGen names!)

It largely follows the same strategy as the LHCb names, but with the important difference of loading from $RAPIDSIM_ROOT/config/particles.dat if present, which is nice if you're using particle in e.g. a conda environment with rapidsim installed.

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 64.58333% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.31%. Comparing base (a2093ca) to head (14b095d).

Files with missing lines Patch % Lines
src/particle/rapidsim/converters.py 48.27% 15 Missing ⚠️
src/particle/rapidsim/__init__.py 83.33% 1 Missing ⚠️
src/particle/rapidsim/data/__init__.py 83.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #756      +/-   ##
==========================================
- Coverage   97.92%   97.31%   -0.62%     
==========================================
  Files          31       35       +4     
  Lines        2559     2607      +48     
==========================================
+ Hits         2506     2537      +31     
- Misses         53       70      +17     

☔ 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.

@admorris
Copy link
Copy Markdown
Contributor Author

To improve the coverage, I should add a test that mocks $RAPIDSIM_ROOT being set

@eduardo-rodrigues
Copy link
Copy Markdown
Member

Hello @admorris, I will look at this next week ...

One immediate reaction is indeed why RapidSim uses non PDG/EvtGen standard names in the 2020s!? It's more complication for absolute zero gain.

The obvious question is whether it would not be simpler in the end to sort the issue at source? In the end they must be using names as labels, right, so should be rather easy? Have you checked this with the authors?

I am not saying I am opposing to having another submodule as we already have for LHCb. Just asking for the record.

Thanks.

@eduardo-rodrigues eduardo-rodrigues self-assigned this Apr 24, 2026
@eduardo-rodrigues eduardo-rodrigues added the ⚙️ enhancement New feature or request label Apr 24, 2026
@admorris
Copy link
Copy Markdown
Contributor Author

Hi Eduardo

That's a fair suggestion.

I just had a go at converting RapidSim's particles.dat to use EvtGen names, and I found 4 instances where the PDG ID does not match 🥲 particles.dat.txt

There are a lot of open PRs and issues in RapidSim which suggest it's not being actively maintained. But I'm sure Greig and Dan are still contactable.

@eduardo-rodrigues
Copy link
Copy Markdown
Member

Hi.

I checked briefly your data file and all 4 misses are for wrongly assigned PDGIDs from RapidSim authors. I could find the correct replacements in https://github.com/scikit-hep/particle/blob/main/src/particle/data/conversions.csv.

I don't know how you and LHCb would like to proceed, but that seems like the main point here, else I am happy to merge this as-is if that helps with support for LHCb.

I see many LHCb analysts still using RapidSim, hence it is useful and should be maintained. If the authors are not active anymore, could the package be taken over by LHCb somehow? I see it is effectively dead, since the last release is from 2019 and the latest merge from 2 years ago - https://github.com/gcowan/RapidSim!

with open("src/particle/rapidsim/data/pdgid_to_rapidsimname.csv", "w") as out_csv:
out_csv.write(
f"# (c) Scikit-HEP project - Particle package data file - pdgid_to_rapidsimname.csv - {date}\n"
)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
)
)
csvfile.write(
"# Auto generated by 'admin/dump_pdgid_to_rapidsimname.py'\n",
)

@@ -0,0 +1,185 @@
# (c) Scikit-HEP project - Particle package data file - pdgid_to_rapidsimname.csv - 2026-04-24
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
# (c) Scikit-HEP project - Particle package data file - pdgid_to_rapidsimname.csv - 2026-04-24
# (c) Scikit-HEP project - Particle package data file - pdgid_to_rapidsimname.csv - 2026-04-24
# Auto generated by 'admin/dump_pdgid_to_rapidsimname.py

"""
lines: list[str] = ["PDGID,STR\n"]
with dat_file.open(encoding="utf-8") as f:
next(f) # skip header line
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There is now an extra line to skip given my added comment, right?

Copy link
Copy Markdown
Member

@eduardo-rodrigues eduardo-rodrigues left a comment

Choose a reason for hiding this comment

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

This looks great and I only made a couple of trivial suggestions.

Another suggestion would be to add somewhere in the docs the link to RapidSim, probably in admin/dump_pdgid_to_rapidsim.py but also in the functions to and from?

I let you decide if you update the docs for this new functionality here or in a follow-up PR.

Last thing: did you then decide what to do overall, I mean given the exchange above? Even if some work is going to be done to revive and modernise RapidSim, it may still take a while before that's available. If helpful we can move on merging and releasing this asap. Just let me know.

Thank you again for the great and clear enhancement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

⚙️ enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants