Skip to content

Add the EfficiencyFilter algorithm and tests running it #322

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.

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

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

jmcarcell
Copy link
Member

@jmcarcell jmcarcell commented May 26, 2025

BEGINRELEASENOTES

  • Add an EfficiencyFilter, an algorithm that will take an EDM4hep collection and return a subset collection with a certain efficiency. The Exact parameter can be used to control whether the efficiency is exactly that number of simply
    each element is kept with Efficiency probability.
  • Add a test that runs the EfficiencyFilter both in the exact version (to check that exactly Efficiency * length elements are saved) and in the non-exact version

ENDRELEASENOTES

It's also an example of a transformer that works on a single arbitrary collection, of which we didn't have any examples before.

See key4hep/k4Reco#27 (comment)

@jmcarcell
Copy link
Member Author

@mahmoudali2 can you try to use this for simulating the efficiency? You would need to build k4FWCore with this pull request and then you can run DDPlanarDigi without building since you wouldn't need key4hep/k4Reco#27. This has the advantage that you can know exactly which hits are lost in DDPlanarDigi since you know which hits remain after the efficiency cut.

@jmcarcell jmcarcell force-pushed the efficiency-filter branch from 1cfdd15 to 99e2faa Compare May 26, 2025 19:58
@jmcarcell jmcarcell force-pushed the efficiency-filter branch from 9873265 to 5a18905 Compare May 27, 2025 11:50
@m-fila
Copy link
Member

m-fila commented May 27, 2025

This won't work with the data-model extensions, right?

@jmcarcell jmcarcell force-pushed the efficiency-filter branch from 4a0fd50 to 18df25e Compare June 12, 2025 15:53
@jmcarcell
Copy link
Member Author

This won't work with the data-model extensions, right?

No, like any algorithm we create here, it can't know about the extensions. It's always the case with any algorithm, no?

@tmadlener
Copy link
Contributor

I am wondering whether we can have a somewhat lightweight pseudo plugin mechanism here that makes it possible to add another type list downstream where extensions are defined. OTOH, not having all the functionality for all possible extensions probably also serves as a good incentive for people to actually go use EDM4hep.

@BrieucF
Copy link
Contributor

BrieucF commented Jun 13, 2025

I am wondering whether we can have a somewhat lightweight pseudo plugin mechanism here that makes it possible to add another type list downstream where extensions are defined. OTOH, not having all the functionality for all possible extensions probably also serves as a good incentive for people to actually go use EDM4hep.

I would argue here that since we have the policy to integrate in EDM4hep new data types only if they have been thoroughly tested, we'd rather provide the needed functionalities to fully integrate extensions into realistic workflows.

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.

5 participants