refactor!: Refactor CKF branch stopper to allow stop and keep tracks #3102
Conversation
andiwand
changed the title
📊: Physics performance monitoring for d8560bfphysmon summary
|
acts-policybot
bot
dismissed
timadye’s stale review
Invalidated by push of 09cfd26
|
I am measuring a speedup of 3x which makes me wonder if that can be real 🤔
|
There was a problem hiding this comment.
Unfortunately, when I run full_chain_itk.py, I get a crash:
Core/include/Acts/Utilities/VectorHelpers.hpp:141: std::array<double, 4> Acts::VectorHelpers::evaluateTrigonomics(const Acts::Vector3&): Assertion `sinTheta != 0 && "VectorHelpers: Vector is parallel to the z-axis " "which leads to division by zero"' failed.
after ~47 events. With main HEAD, it runs OK, at least to >90 events. Of course that difference could just be bad luck.
Hm that looks unrelated. I can try to see if this is easily fixable but this should go into another PR in any case. |
|
Those fix the problems I had, but my attempt to merge #3164 with this (#3102) led to a crash on the first event: Maybe I merged them badly. Did you try with both PRs? |
|
For 1000 ttbar+PU200 events, with 8 threads, I get an overall speedup 9.1→8.2 s/event (x1.10) for the whole job, and in the |
|
Some more plots with interesting features: |
|
Just to be sure: these plots come from Athena reconstruction? And all of them are ttbar PU 200?
I believe the increase in efficiency comes from #3164. I found a spot where we stop the branch but never stored the track. Speedup 1.4 is great! I am surprised that it is so different between ODD Acts and ITk Athena-Acts. Maybe the previous branch aborter was already more effective than the ODD Acts one because of the detector geometry. I guess you are using the same holes/outlier cuts as before on Athena main? |
No, they are ACTS stand-alone with the ITk. All with ttbar PU 200. I can't easily test it in Athena until the PRs are merged - and even then, the
That is η dependent. Overall, the efficiency is larger in more η regions than regions where it is smaller.
right. Most ttbar PU 200 tracks are low pT.
👍
Cool! I'll test this with #3164 on its own to separate the effect of the two PRs.
Did you check this is still the case? You expressed some doubt about the x3 number before, and the lack of spread in times for different events is a little suspicious. I assume your plot before was for the
This is acts #3102 vs acts |
I will run it again. The output seems to have changed significantly with 2b068ea which is unexpected but the change looks more correct than what was there before. One suspicion I have is that we end up in the Solenoid quite often in the ODD and just waste a lot of time propagating there. This is prohibited in a lot of cases not by stopping the finding earlier. |
Speedup is still significant - seems to be real. Here are some stats from the track finding which gives a little bit more insight. main changed stopped branches is ~10x |
github-actions
bot
added
Infrastructure
|
again for the ITk:
|
acts-project-service
added
Breaks Athena build
…cts-project#3102) Changes the CKF branch stopper to allow keeping the tracks after the branch is stopped. This is useful if a track already collected enough measurements but then starts to accumulate holes. This change should allow us to stop branches more aggressively because we don't have to be worried anymore to throw out valid track candidates. closes - acts-project#2967 blocked by - acts-project#3164 - acts-project#3163
After removing this in #3102 I reintroduce this functionality here to reduce the changes to a minimum. After #3102 we see some bigger tack finding changes than expected which I suspect are coming from the fact that we effectively stopped most of the branches when reaching a measurement surface. Having this callback will also be necessary if we want to allow the user to count holes/outliers/measurements in specific geometry regions by themselves.
…cts-project#3102) Changes the CKF branch stopper to allow keeping the tracks after the branch is stopped. This is useful if a track already collected enough measurements but then starts to accumulate holes. This change should allow us to stop branches more aggressively because we don't have to be worried anymore to throw out valid track candidates. closes - acts-project#2967 blocked by - acts-project#3164 - acts-project#3163
…oject#3172) After removing this in acts-project#3102 I reintroduce this functionality here to reduce the changes to a minimum. After acts-project#3102 we see some bigger tack finding changes than expected which I suspect are coming from the fact that we effectively stopped most of the branches when reaching a measurement surface. Having this callback will also be necessary if we want to allow the user to count holes/outliers/measurements in specific geometry regions by themselves.
Changes the CKF branch stopper to allow keeping the tracks after the branch is stopped. This is useful if a track already collected enough measurements but then starts to accumulate holes.
This change should allow us to stop branches more aggressively because we don't have to be worried anymore to throw out valid track candidates.
closes
blocked by
CKF#3164full_chain_itk.py#3163