Skip to content
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

TruthParticle Update and Efficiency Measurements, main branch (2024.05.08.) #582

Closed

Conversation

krasznaa
Copy link
Member

This is the thing that I was teasing @beomki-yeo and @SylvainJoube with for a few days now. 🤔

Basically, I believe that what we desperately need is a well defined reconstruction and truth EDM in the project. With those in place, doing the physics performance studies should become easier. Now... the ultimate form of the EDM will be an SoA one, that's not up for debate. But for now, as a quick hack, I want to introduce traccc::particle_container_types as:

using particle_container_types = container_types<particle, measurement>;

So that the container would neatly associate (true) measurements with the particles in a "container form".

With this setup, traccc::track_candidate_container_types and this traccc::particle_container_types container become directly comparable. And this is what this PR's toy class (traccc::performance::track_finding_analysis) does. Right now it only does some silly stuff that I used for debugging purposes, but a class with that sort of API should be able to perform some reconstruction efficiency measurements. 🤔

Eventually of course I'll want to have a "truth EDM" that is:

  • a collection of particles with the measurement indices that belong to those particles;
  • a collection of measurements with the cell indices that belong to those measurements;
  • a collection of cells.

Note that we have all the information in our CSV files for building this sort of EDM in memory. 🤔 We shouldn't be running clusterization as part of some helper classes, like we do currently in: https://github.com/acts-project/traccc/blob/main/io/src/mapper.cpp#L227-L230 Instead, we'll just need to write a relatively simple piece of code that establishes the particle->measurement->cell connections, based on the information that we have in the CSV files. 🤔

I intend to keep this PR in a draft status for a bit. I hope to be able to produce some simple efficiency plots with this sort of code until next Thursday. And then, once I'm back from the HSF workshop, we can see about doing all of this in a properly clean way. 🤔

@krasznaa krasznaa force-pushed the TruthParticleUpdate-main-20240508 branch from ee05523 to b6612b7 Compare May 12, 2024 19:32
@krasznaa
Copy link
Member Author

So, the code is now producing some track finding efficiency results.

But let's start at the beginning. I switched to doing the truth measurement <-> reco measurement matching a little differently. Instead of comparing their properties "percentage-wise", I now rather check whether the measurements are within certain absolute distance to each other. (On top of being on the same surface of course.) As that is just more justified in this case. (We're not looking for the floating-point precision of our code in this case, but the physics performance of it. For which absolute value comparisons seem more reasonable.)

You can absolutely disagree with the 1 millimeter distance that I allow in the code currently. Unfortunately when I go to a smaller value, the final efficiencies drop drastically. So we'll have to review the measurement creation code a bit... 🤔

With this code, on a large-statistics 100 GeV muon sample, I get:

track_finding_eff_eta
track_finding_eff_phi

We will have plenty to understand... 🤔 But at least we now have a sense of what the ODD reconstruction is doing. 😉

@krasznaa
Copy link
Member Author

Also note that when I run:

./bin/traccc_seq_example --detector-file=geometries/odd/odd-detray_geometry_detray.json --grid-file=geometries/odd/odd-detray_surface_grids_detray.json --use-detray-detector --digitization-file=geometries/odd/odd-digi-geometric-config.json --input-directory=odd/geant4_10muon_100GeV --input-events=100 --check-performance

on the files that we have in our v7 data, I get an even weirder plot. 😕

track_finding_eff_eta_v7

So something is a bit fishy around the simulations as well. (Since this plot should just be lower-statistics compared to the geant4_100muon_100GeV/ one that I made earlier. But the shape is just very different...)

Though now that I think about it, it's probably because of a mismatch in the digitization parameters used. 🤔 (Since the measurement matching code requires the uncertainty on the measurements to match as well. Which only happens if the digitization parameters are used consistently between Acts's simulation and this project's reconstruction.) But it seems that some of the strip measurements just don't match up in this latter plot. 😕

@krasznaa krasznaa force-pushed the TruthParticleUpdate-main-20240508 branch from e1ea7f7 to 5251c34 Compare May 14, 2024 08:23
@SylvainJoube
Copy link
Contributor

Great! I think it would also be a very interesting topic for an Acts Parallelization Meeting roundtable, although I doubt you will be able at attend the next meeting (May 17) as you will likely still be in Hamburg since the HSF workshop ends at about 12:45 that day.

Thanks for the ping by the way :)

@krasznaa krasznaa force-pushed the TruthParticleUpdate-main-20240508 branch from 5251c34 to e176791 Compare May 22, 2024 13:51
@krasznaa
Copy link
Member Author

So... good news and bad news.

  • The good news is that we were right;
  • The bad news is that the seed-finding efficiency of the code on the ODD is pretty terrible.
    • But another good news is that the track finding efficiency is apparently pretty great on top of this terrible seed finding efficiency. 😉 (As that is still the same as I posted earlier, looking pretty much like the seeding efficiency.)

seed_eff

I'll turn this PR into digestible PRs in the coming days, so that it could be merged in. Then we can start with the optimisation of the seeding options. 😉

krasznaa added 9 commits June 26, 2024 14:40
It performs the matching based on the measurements associated
with the truth and reconstructed particles.
While loosening the requirement on the variance of the measurements,
so that they would still be considered the same measurement. This
is because "true measurements" and the ones reconstructed by our
code tend to be just a little different for the pixels.
While changing how the truth<->reco track matching would be done
exactly by the class.

Also removed the currently ineffective tools from
traccc_seq_example.
So that it would be easier to introduce additional performance
measurements in the next step(s).
So that it would overlay the seed-finding and track-finding
efficiencies on top of each other. In a super hard-coded way.
@krasznaa krasznaa force-pushed the TruthParticleUpdate-main-20240508 branch from 947a010 to 2637ab0 Compare June 26, 2024 12:40
@krasznaa
Copy link
Member Author

With the traccc integration into Acts providing way-way better performance monitoring than what we should put into this project itself, let's abandon this update. And eventually we should probably remove most of the physics performance measuring code from the repository. It's enough to maintain that sort of thing in Acts...

@krasznaa krasznaa closed this Aug 14, 2024
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.

2 participants