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

feat: Add ability to skip grid bins in SP grouping and add ITk "fast" tracking config for seeding #2166

Merged
merged 38 commits into from
Aug 7, 2023

Conversation

LuisFelipeCoelho
Copy link
Member

@LuisFelipeCoelho LuisFelipeCoelho commented May 30, 2023

This PR adds the ITk fastTracking configuration for the seeding, which includes:

  • A new feature in binnedSPGroup that allow us to skip z bins for middle SPs in the search for neighbours SPs
  • A new flag in the python ITk configuration for high occupancy seeding

@LuisFelipeCoelho LuisFelipeCoelho added 🚧 WIP Work-in-progress Component - Core Affects the Core module Component - Examples Affects the Examples module labels May 30, 2023
@LuisFelipeCoelho LuisFelipeCoelho added this to the next milestone May 30, 2023
@codecov
Copy link

codecov bot commented May 30, 2023

Codecov Report

Merging #2166 (ce24ad3) into main (3f4cbab) will decrease coverage by 0.08%.
The diff coverage is 0.00%.

❗ Current head ce24ad3 differs from pull request most recent head aa5f33d. Consider uploading reports for the commit aa5f33d to get more accurate results

@@            Coverage Diff             @@
##             main    #2166      +/-   ##
==========================================
- Coverage   49.65%   49.57%   -0.08%     
==========================================
  Files         453      451       -2     
  Lines       25534    25464      -70     
  Branches    11708    11690      -18     
==========================================
- Hits        12679    12624      -55     
- Misses       4574     4576       +2     
+ Partials     8281     8264      -17     
Files Changed Coverage Δ
Core/include/Acts/Seeding/BinnedSPGroup.hpp 0.00% <0.00%> (ø)
Core/include/Acts/Seeding/BinnedSPGroup.ipp 0.00% <0.00%> (ø)

... and 16 files with indirect coverage changes

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

@paulgessinger
Copy link
Member

Is there any way to make this non-breaking by using default arguments?
We have to start being a little careful when breaking backwards compatibility
, especially in the seeding since folks have reported this being a reason holding them back form updating the library.

@LuisFelipeCoelho LuisFelipeCoelho changed the title feat!: ITk fast tracking configuration for Seeding feat: ITk fast tracking configuration for Seeding May 31, 2023
@LuisFelipeCoelho
Copy link
Member Author

I have discussed with @paulgessinger and I realised that this is actually not a breaking change because it just adds new config parameters that are defaulted to something that won't change the performance or the build

Core/include/Acts/Seeding/BinnedSPGroup.hpp Outdated Show resolved Hide resolved
Core/include/Acts/Seeding/BinnedSPGroup.ipp Outdated Show resolved Hide resolved
Core/include/Acts/Seeding/SeedFinderOrthogonal.ipp Outdated Show resolved Hide resolved
@CarloVarni
Copy link
Collaborator

@LuisFelipeCoelho any news on this?

@github-actions github-actions bot added the Component - Plugins Affects one or more Plugins label Jul 4, 2023
@LuisFelipeCoelho
Copy link
Member Author

I have completely removed the variables

bool fastTrackingCut = false;
float fastTrackingRMin = 50. * Acts::UnitConstants::mm;
float fastTrackingCotThetaMax = 1.5;

and replaced by a delegate (#2326).

I also replaced the name fastSeeding in the python config to highOccupancyConfig

@github-actions github-actions bot removed the Component - Plugins Affects one or more Plugins label Jul 28, 2023
@LuisFelipeCoelho LuisFelipeCoelho changed the title feat: ITk fast tracking configuration for Seeding feat: skip grid bins in SP grouping + ITk "fast" tracking config for Seeding Jul 28, 2023
@LuisFelipeCoelho LuisFelipeCoelho changed the title feat: skip grid bins in SP grouping + ITk "fast" tracking config for Seeding feat: skip grid bins in SP grouping and ITk "fast" tracking config for Seeding Jul 28, 2023
Copy link
Member

@paulgessinger paulgessinger 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. Has also already seen extensive other review.

@paulgessinger paulgessinger changed the title feat: skip grid bins in SP grouping and ITk "fast" tracking config for Seeding feat: Add ability to skip grid bins in SP grouping and add ITk "fast" tracking config for seeding Aug 3, 2023
@paulgessinger paulgessinger modified the milestones: next, v28.1.0 Aug 7, 2023
@kodiakhq kodiakhq bot merged commit 14b0cea into acts-project:main Aug 7, 2023
54 checks passed
@github-actions github-actions bot removed the automerge label Aug 7, 2023
@LuisFelipeCoelho LuisFelipeCoelho deleted the fastTrackITk branch August 9, 2023 08:06
paulgessinger pushed a commit that referenced this pull request Aug 10, 2023
Protection for large number of bins which is set by default to `std::numeric_limits<int>::max()`

This comes from PR #2166, I am splitting the changes into smaller PRs.
kodiakhq bot pushed a commit that referenced this pull request Nov 24, 2023
This PR adds an option to include experimental specific cuts to discard bottom-middle dublets in a certain (r, eta) region of the detector.

This PR is related to the implementation of the ITk "fast tracking" configuration from PR #2166
(I am splitting the changes to make it more organised @CarloVarni @noemina)

For the integration with Athena we will need to implement something like that in Athena:
```
m_finderCfg.experimentCuts.connect(
	[](const void*, const float& bottomRadius, const float& cotTheta) -> bool {

        if (bottomRadius < fastTrackingRMin and
               (cotTheta > fastTrackingCotThetaMax or
                cotTheta < -fastTrackingCotThetaMax)) {
             return false;
        }
        return true;
}
```

where:

```
float fastTrackingRMin = 50. * Acts::UnitConstants::mm;
float fastTrackingCotThetaMax = 1.5;
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component - Core Affects the Core module Component - Examples Affects the Examples module Seeding Track Finding
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants