refactor: time optimization for calculation of min and max r variable range

Open LuisFelipeCoelho opened this issue 3 years ago • 5 comments

This PR changes the position of rMiddleSPRange calculation and adds a break when SPs are sorted in r

Sep 27 '22 13:09 LuisFelipeCoelho

Codecov Report

Merging #1558 (7e80346) into main (f6f0e1f) will decrease coverage by 0.00%. The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main    #1558      +/-   ##
==========================================
- Coverage   48.65%   48.65%   -0.01%     
==========================================
  Files         381      381              
  Lines       20777    20780       +3     
  Branches     9517     9518       +1     
==========================================
  Hits        10110    10110              
- Misses       4096     4099       +3     
  Partials     6571     6571

Impacted Files	Coverage Δ
Core/include/Acts/Seeding/SeedFinder.hpp	`0.00% <ø> (ø)`
Core/include/Acts/Seeding/SeedFinder.ipp	`0.00% <0.00%> (ø)`

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

Sep 27 '22 13:09 codecov[bot]

Can you consider using a pair or separate argument for the min/max values, instead of a Vector2? I don't think using Vector2 brings any tangible benefit, and it might be a bit confusing what it means.

Oct 03 '22 07:10 paulgessinger

Can you consider using a pair or separate argument for the min/max values, instead of a Vector2? I don't think using Vector2 brings any tangible benefit, and it might be a bit confusing what it means.

Actually, as Andi mentioned last developer meeting, it looks like a good use case for Acts::Range1D from the utilities folder.

I also noticed that there is this `SeedConfirmationRangeConfig.hpp that also defines two ranges as just min and max values that could also be refactored, however not necessarily im this PR.