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

fix: ParticleID collision in Geant4 #2039

Merged
merged 16 commits into from
Apr 21, 2023

Conversation

benjaminhuth
Copy link
Member

@benjaminhuth benjaminhuth commented Apr 18, 2023

This attempts to fix particle collisions in Geant4. These occur I think when the bits in the ActsFatras::Barcode are not enough to capture all results. This way it could happen that multiple particles where associated with a single ID:

image

I think the correct fix for this to check for collision and ignoring particles that collide. Furthermore, the barcode generation is improved, so that collisions should happen less frequently.

Since this happens quite frequently with default settings for Geant4+Pythia8(ttbar), I think all so far generated events using this configuration are suffering from this problem

Benjamin Huth and others added 2 commits April 18, 2023 17:56
@benjaminhuth benjaminhuth added the 🚧 WIP Work-in-progress label Apr 18, 2023
@benjaminhuth benjaminhuth added this to the next milestone Apr 18, 2023
@andiwand
Copy link
Contributor

tagging @Corentin-Allaire as this might impact the ambi performance eval?

@codecov
Copy link

codecov bot commented Apr 18, 2023

Codecov Report

Merging #2039 (822abb8) into main (74df04d) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #2039   +/-   ##
=======================================
  Coverage   49.83%   49.83%           
=======================================
  Files         420      420           
  Lines       23962    23962           
  Branches    10859    10859           
=======================================
  Hits        11942    11942           
  Misses       4390     4390           
  Partials     7630     7630           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@andiwand andiwand left a comment

Choose a reason for hiding this comment

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

looks good from my side

I wonder if we can optimize the barcode or the utilization of the barcode in the G4 translation so we avoid overflows for ttbar

@Corentin-Allaire
Copy link
Contributor

tagging @Corentin-Allaire as this might impact the ambi performance eval?

It definitely could affect it, if we have multiple track with the same id then one of them will be treated as a duplicate

@github-actions
Copy link

github-actions bot commented Apr 18, 2023

📊 Physics performance monitoring for 411a5e6

Full report
Seeding: seeded, truth estimated, orthogonal
CKF: seeded, truth smeared, truth estimated, orthogonal
IVF: seeded, truth smeared, truth estimated, orthogonal
Ambiguity resolution: seeded, orthogonal
Truth tracking
Truth tracking (GSF)

Vertexing

Vertexing vs. mu
IVF seeded

IVF truth_smeared

IVF truth_estimated

IVF orthogonal

Seeding

Seeding seeded

Seeding truth_estimated

Seeding orthogonal

CKF

CKF seeded

CKF truth_smeared

CKF truth_estimated

CKF orthogonal

Ambiguity resolution

seeded

Truth tracking (Kalman Filter)

Truth tracking

Truth tracking (GSF)

Truth tracking

@benjaminhuth benjaminhuth marked this pull request as draft April 18, 2023 18:23
@benjaminhuth
Copy link
Member Author

benjaminhuth commented Apr 18, 2023

Actually there are quite a lot of collisions in one event:

20:47:59    Geant4Simula   WARNING   2718782 particles missing in the output data because of particle ID collissions

@Corentin-Allaire
Copy link
Contributor

Actually there are quite a lot of collisions in one event:

20:47:59    Geant4Simula   WARNING   2718782 particles missing in the output data because of particle ID collissions

That number seems a bit too high, for how many event is that ?

@andiwand
Copy link
Contributor

andiwand commented Apr 18, 2023

That number seems a bit too high, for how many event is that ?

I dont actually know but it might be plausible considering all the secondary particles? I once use G4 with 100 muon event and got like 10k secondaries

@benjaminhuth
Copy link
Member Author

That number seems a bit too high, for how many event is that ?

Its for one event, but without post selection. And probably you need to divide this number by two, because right now it counts initial and final particles.

@Corentin-Allaire
Copy link
Contributor

That number seems a bit too high, for how many event is that ?

Its for one event, but without post selection. And probably you need to divide this number by two, because right now it counts initial and final particles.

I just run the job with the default selector and I see 0 overlap so this probably doesn't change much in the end ?

@asalzburger
Copy link
Contributor

looks good from my side

I wonder if we can optimize the barcode or the utilization of the barcode in the G4 translation so we avoid overflows for ttbar

Sure, if we can change the barcode behaviour to be safe against this, let's do it!

@asalzburger
Copy link
Contributor

Since this happens quite frequently with default settings for Geant4+Pythia8(ttbar), I think all so far generated events using this configuration are suffering from this problem

Can you specify what this means? Which particles are disregarded?
And how does this free enough space in the barcode bits?

@benjaminhuth benjaminhuth marked this pull request as ready for review April 20, 2023 08:16
Copy link
Contributor

@andiwand andiwand left a comment

Choose a reason for hiding this comment

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

thanks for turning this around so quickly 👍

looks good - just two questions

@benjaminhuth
Copy link
Member Author

benjaminhuth commented Apr 20, 2023

Can you specify what this means? Which particles are disregarded? And how does this free enough space in the barcode bits?

Finally, I have some plots @asalzburger
That is 1 event ttbar+Geant4. But I still don't understand every detail of the results.

image

image

@asalzburger asalzburger added automerge and removed 🚧 WIP Work-in-progress labels Apr 21, 2023
@benjaminhuth benjaminhuth added 🚧 WIP Work-in-progress and removed automerge labels Apr 21, 2023
@asalzburger
Copy link
Contributor

@benjaminhuth - can you fix the format, please ?

@benjaminhuth
Copy link
Member Author

Yeah and I need to do some clean-up still

Benjamin Huth added 3 commits April 21, 2023 11:39
This reverts commit ad6edd7.
This reverts commit dbad4c2.
@benjaminhuth benjaminhuth added Component - Examples Affects the Examples module Impact - Major Significant bug and/or affects a lot of modules and removed 🚧 WIP Work-in-progress Impact - Major Significant bug and/or affects a lot of modules labels Apr 21, 2023
@benjaminhuth
Copy link
Member Author

@asalzburger The CI should run through now, could you reapprove?

@kodiakhq kodiakhq bot merged commit f2d8c25 into acts-project:main Apr 21, 2023
37 checks passed
paulgessinger pushed a commit to paulgessinger/acts that referenced this pull request Apr 25, 2023
This attempts to fix particle collisions in Geant4. These occur I think when the bits in the `ActsFatras::Barcode` are not enough to capture all results. This way it could happen that multiple particles where associated with a single ID:

![image](https://user-images.githubusercontent.com/37871400/232845631-9b36bcb3-47a1-49bf-91a2-313305d7314b.png)

I think the correct fix for this to check for collision and ignoring particles that collide. Furthermore, the barcode generation is improved, so that collisions should happen less frequently.

Since this happens quite frequently with default settings for Geant4+Pythia8(ttbar), I think all so far generated events using this configuration are suffering from this problem
@benjaminhuth benjaminhuth deleted the fix/particle-id-collision branch May 10, 2023 07:15
@paulgessinger paulgessinger modified the milestones: next, v26.0.0 May 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component - Examples Affects the Examples module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants