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: change name of numPhiNeighbors in the grid #1220

Conversation

LuisFelipeCoelho
Copy link
Member

As @robertlangenberg and @noemina have addressed in PR #1167 and issue #1172, numPhiNeighbors for Grid and BinFinder don't mean the same. The numPhiNeighbors for the Grid is the number of phiBin neighbors that should cover the full deflection of a minimum pT particle, while numPhiNeighbors in the BinFinder is the number of phi bin neighbors (including the current one) that are used to search for SPs.

This PR renames numPhiNeighbors for the Grid to phiBinDeflectionCoverage and allows for even numbers of phiBins.

@LuisFelipeCoelho LuisFelipeCoelho added Component - Core Affects the Core module Improvement Changes to an existing feature Impact - Minor Nuissance bug and/or affects only a single module labels Mar 31, 2022
@LuisFelipeCoelho LuisFelipeCoelho added this to the next milestone Mar 31, 2022
@codecov
Copy link

codecov bot commented Mar 31, 2022

Codecov Report

Merging #1220 (fdcc939) into main (b7d70c4) will not change coverage.
The diff coverage is 0.00%.

@@           Coverage Diff           @@
##             main    #1220   +/-   ##
=======================================
  Coverage   47.86%   47.86%           
=======================================
  Files         372      372           
  Lines       19443    19443           
  Branches     9151     9151           
=======================================
  Hits         9306     9306           
  Misses       3806     3806           
  Partials     6331     6331           
Impacted Files Coverage Δ
Core/include/Acts/Seeding/SpacePointGrid.hpp 0.00% <ø> (ø)
Core/include/Acts/Seeding/SpacePointGrid.ipp 0.00% <0.00%> (ø)

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

Copy link
Contributor

@robertlangenberg robertlangenberg left a comment

Choose a reason for hiding this comment

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

please address comments

@robertlangenberg robertlangenberg merged commit ae4ac11 into acts-project:main Apr 5, 2022
@LuisFelipeCoelho LuisFelipeCoelho deleted the phiBin-deflection-coverage branch April 5, 2022 15:19
paulgessinger pushed a commit to paulgessinger/acts that referenced this pull request Apr 11, 2022
* change numPhiNeighbors in grid creation step

* set default value

* addressing comments
@paulgessinger paulgessinger modified the milestones: next, v19.0.0 May 10, 2022
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 Impact - Minor Nuissance bug and/or affects only a single module Improvement Changes to an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants