feat: Hough Transform first implementation

[jahreda]

This is a first implementation of a Hough Transform into the ACTS code (also my first PR into ACTS). I think I have found some decent solutions for all the problems and questions I had before. The HT implementation is largely based on the ATLAS EF Tracing Hough Transform. The implementation is incomplete in that it only has example region selection and z sub-region slicing, but this can be implemented for specific geometries.

Currently the code exists as an Example, so I would like some feedback as to where best to put this (from previous discussions it wasn't fully clear).

Just to show it's working, from 4 muon events in the barrel (narrow phi region): 22:36:33 SeedingPerfo INFO Efficiency (nMatchedParticles / nAllParticles) = 0.994784 22:36:33 SeedingPerfo INFO Fake rate (nUnMatchedSeeds / nAllSeeds) = 0.617954 22:36:33 SeedingPerfo INFO Duplication rate (nDuplicatedMatchedParticles / nMatchedParticles) = 0.997378 22:36:33 SeedingPerfo INFO Average number of duplicated seeds ((nMatchedSeeds - nMatchedParticles) / nMatchedParticles) = 26.7891 22:36:33 SeedingPerfo INFO Wrote performance plots to '/afs/cern.ch/work/j/jahreda/acts_april28/acts/data/output_four_muons_hough/performance_seeding_hists.root:/' 22:36:33 RootTrackPar INFO Write estimated parameters from seed to tree 'estimatedparams' in '/afs/cern.ch/work/j/jahreda/acts_april28/acts/data/output_four_muons_hough/estimatedparams.root' 22:36:34 Sequencer INFO Processed 10000 events in 79.477941 s (wall clock) 22:36:34 Sequencer INFO Average time per event: 77.954230 ms/event

[codecov[bot]]

Codecov Report

Merging #1305 (33d8de8) into main (a11ef61) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1305   +/-   ##
=======================================
  Coverage   48.95%   48.95%           
=======================================
  Files         397      397           
  Lines       21509    21509           
  Branches     9806     9806           
=======================================
  Hits        10530    10530           
  Misses       4181     4181           
  Partials     6798     6798           

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

[jahreda]

Thanks, Robert. It's great to have someone taking a look. A few questions:

1. Isn't the code configurable? For example:

• Number of layers: https://github.com/jahreda/acts/blob/jahredaHTDev/Examples/Algorithms/TrackFinding/include/ActsExamples/TrackFinding/HoughTransformSeeder.hpp#L133
• Binning (number of bins): https://github.com/jahreda/acts/blob/jahredaHTDev/Examples/Algorithms/TrackFinding/include/ActsExamples/TrackFinding/HoughTransformSeeder.hpp#L146 and range: (https://github.com/jahreda/acts/blob/jahredaHTDev/Examples/Algorithms/TrackFinding/include/ActsExamples/TrackFinding/HoughTransformSeeder.hpp#L141)

Were you thinking this has to be done in some other ways? If so, can you point me to how you'd like me to set up the config?

2. Moving this to Core makes sense - but do you have examples (of the seeding or otherwise) for how you suggest doing that and also providing an "example"? Outside of the functions here: https://github.com/jahreda/acts/blob/jahredaHTDev/Examples/Algorithms/TrackFinding/include/ActsExamples/TrackFinding/DefaultHoughFunctions.hpp

I think nothing is actually geometry dependent.

3. I'm also curious what you mean by "next to the Seeding"

4. Finally, I think I maybe misunderstood what Andi was suggesting for measurements or maybe I'm simply not remembering. You are right that the code does loop over all the measurements. I think no matter what one has to loop over all them at least once. What I do is loop over them and pull out just a handful of pieces of information: layer, phi, r, z and index. This is done here: https://github.com/jahreda/acts/blob/jahredaHTDev/Examples/Algorithms/TrackFinding/src/HoughTransformSeeder.cpp#L210 When we loop over the bins in a layer we can then quickly check if we are in the right layer or not. And if not we can skip the measurement: https://github.com/jahreda/acts/blob/jahredaHTDev/Examples/Algorithms/TrackFinding/src/HoughTransformSeeder.cpp#L309

In principle we could speed that up by storing the above separately by layer. Is that what you propose? Or something else?

Best Jahred

[jahreda]

Hi @robertlangenberg

Just pinging this thread to see if you had a chance to look at my replies. I have been on vacation, traveling and then dealing with a sick family so I haven't made our Tuesday meeting recently. And this coming week I'm home with my son on Tuesday, but maybe best to iterate here, anyway.

[jahreda]

I pushed a significant number of updates. The only major things I think left (aside from the python bindings) are understanding how best to provide the inputs to the algorithm. It's not clear to me how best to do that as part of ActsExamples::Sequencer, so for now I've left the SP and measurement duality and just moved the functions to create the internal structure

[tboldagh]

It seems we have hit the issue with lack of "core" SpacePoint interface. To make a progress I suggest:

1. merge HT as an example algorithm.
2. work out what to do with the SP (SP collection presumably) - with high priority
3. move the HT to core (presumably after seeing some early results)

@paulgessinger @asalzburger - what do you think?

[jahreda]

Thank you for the follow-up, @tboldagh. I think your plan makes sense if @paulgessinger and @asalzburger agree, and in that case hopefully they can start another review so we can proceed

[jahreda]

Thanks, Andi! I implemented the simple ones. I did also have some questions, especially (but not only) on the grid implementation, since it's not obvious to me how best to do this. A simple example would help!

[jahreda]

Hi Andi - before figuring out those conflicts, I'm trying to understand if you want to follow up on anything else. I think from what I saw the use of grid is the only real open item? I'm still not sure how best to do that, but it also sounded like you agreed to move ahead as long as we put that on a to-do list?

[tboldagh]

Hi Andi - before figuring out those conflicts, I'm trying to understand if you want to follow up on anything else. I think from what I saw the use of grid is the only real open item? I'm still not sure how best to do that, but it also sounded like you agreed to move ahead as long as we put that on a to-do list?

Actually before moving HT to the core we can fix theses issues in smaller PRs while still in examples.

[asalzburger]

Yeah, indeed, the conflicts look rather minor, so if we could resolve them, I would put the HT in. Then, following what we are doing lately, we can shift the code over into Core using the Experimental namespace to indicate that this is not yet production code and might change.

[github-actions[bot]]

:bar_chart: Physics performance monitoring for 33d8de88ad74780778fc27ec770e665c9335c848

:red_square: ERROR The result has missing elements! This is likely a physmon job failure

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

:x: Seeding truth_smeared

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

[jahreda]

@asalzburger I merged my code (required a bit of tinkering) and resolved all conversations so that things can be merged (even if there's more to do as we discussed). I think the remaining blocker is that Robert has requested changes - I guess you have the power to remove them?

[asalzburger]

Updating and merging when pipeline succeeds.

[asalzburger]

clang-tidy is failing, so what I suggest I will do tomorrow is:

• merge this in, followed by a PR that fixes the clang-tidy complaints

[andiwand]

I can also fix the issues if you like @asalzburger but I fear I have not permissions to push on this branch

[asalzburger]

That's why I suggested to merge it & create a hotfix PR that goes in right after.

[andiwand]

That's why I suggested to merge it & create a hotfix PR that goes in right after.

sounds good. lets do that tomorrow. I will create a branch based on this one and we can merge one after the other so nobody will be interrupted by failing ci

[andiwand]

clang tidy fixes ready here https://github.com/acts-project/acts/pull/1691