fix: Bug when sorting the SP in SeedFinder

[LuisFelipeCoelho]

In PR #1084 I mentioned that we were seeing a degradation in tracking efficiency when sorting the SP in cotangent of theta in the SeedFinder. However I realised that I was sorting only the linCircleVec vector in SeedFinder and the compatBottomSP and compatTopSP vectors should also be sorted.

This PR solves the bug and now the efficiency and the output should be the same as without the sorting.

@paulgessinger @robertlangenberg @noemina

[paulgessinger]

Fantastic!

Does that mean we could get rid of enableCutsForSortedSP again?

[codecov]

Codecov Report

Merging #1143 (f02414d) into main (be5fec4) will decrease coverage by 0.02%.
The diff coverage is 0.00%.

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

@@            Coverage Diff             @@
##             main    #1143      +/-   ##
==========================================
- Coverage   47.79%   47.77%   -0.03%     
==========================================
  Files         360      360              
  Lines       18600    18608       +8     
  Branches     8770     8770              
==========================================
  Hits         8890     8890              
- Misses       3658     3666       +8     
  Partials     6052     6052              

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

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

[LuisFelipeCoelho]

I realised that the output is still not the same, so I will have to investigate this a bit further before removing the enableCutsForSortedSP

[LuisFelipeCoelho]

I believe that now we can get rid of enableCutsForSortedS, or we can keep it and add a python-level test that runs the seeding with the cotTheta sorting.

[LuisFelipeCoelho]

One alternative would be to create an index vector and sort this vector based on cotTheta values from linCircleVec. Then create a new vector of pointers newVec, fill it with the new order from the index vector, and replace vec with the new one:

std::vector<size_t> idx(vec.size());
std::iota(idx.begin(), idx.end(), 0);

std::sort(idx.begin(), idx.end(),
	[&linCircleVec](size_t i1, size_t i2) {
	return linCircleVec[i1].cotTheta < linCircleVec[i2].cotTheta;
});

std::vector<const InternalSpacePoint<external_spacepoint_t>*> newVec;
for (size_t i = 0; i<vec.size();i++) {
	newVec.push_back(vec[idx[i]]);
}
vec = newVec;

@paulgessinger @robertlangenberg @noemina

[paulgessinger]

Can't we just have transformCoordinates take the spacepoints as non-const?

Right now we have

template <typename external_spacepoint_t>
void transformCoordinates(
    std::vector<const InternalSpacePoint<external_spacepoint_t>*>& vec,
    const InternalSpacePoint<external_spacepoint_t>& spM, bool bottom,
bool enableCutsForSortedSP, std::vector<LinCircle>& linCircleVec);

Does transformCoordinates get executed on the same vec concurrently? Otherwise, maybe this could be changed to something like std::vector<InternalSpacePoint<external_spacepoint_t>*>&?

[noemina]

Does transformCoordinates get executed on the same vec concurrently? Otherwise, maybe this could be changed to something like std::vector<InternalSpacePoint<external_spacepoint_t>*>&?

I don't think this should happen. @robertlangenberg, can you please confirm it?

[noemina]

Changes look good to me. Approving.