fix seeder to reach 21.9 performances

assigned to @pagessin

added 3 commits

• 9a34290e - Change variable values
• d1199efa - fix typo
• ad2990df - Change condition to match 21.9 one (increase perf)

Compare with previous version

I have overwrite the space-points from a muon sample of R22 by space-point of a muon sample of R21.9 in order to check if the new seeder of 21.9 and the 22 one find the exact same seeds.

After the above fix, both seeders find the same seeds.

added 2 commits

• e14788e7 - revert function
• 714bc630 - std::abs instead of fabs

Compare with previous version

added 1 commit

• 66ec1bad - fix indentation

Compare with previous version

added 1 commit

• 6a8966b4 - fix indentation again

Compare with previous version

added 1 commit

• 85809be5 - add namespace to sqrt

Compare with previous version

added 2 commits

• fe984bc8 - indentation
• 75f07414 - make it pretty

Compare with previous version

As a tip, you don't need to push after each individual commit. As the MR is still WIP, there's no fundamental issue but should the WIP be lifted, each push would trigger a build and run the CI tests. It's preferable in that case to push only once the all sets of commits addressing the MR comments. :)

marked this merge request as draft

marked this merge request as ready

changed title from WIP: fix seeder to reach 21.9 performances to fix seeder to reach 21.9 performances

added 2 commits

• 67f87368 - remove line for consistency
• 3a7a7590 - fix conflict

Compare with previous version

resolved all threads

WARNING: big files (>100K) are found in the changeset

144K in file InnerDetector/InDetRecTools/SiSpacePointsSeedTool_xk/src/ITkSiSpacePointsSeedMaker.cxx

This merge request affects 2 packages:

• InnerDetector/InDetRecEvent/SiSPSeededTrackFinderData
• InnerDetector/InDetRecTools/SiSpacePointsSeedTool_xk

Affected files list will not be printed in this case

Adding @goetz ,@amorley ,@gavrilen ,@sroe ,@jsandesa as watchers

added InnerDetector master review-pending-level-1 labels

CI Result SUCCESS (hash 3a7a7590)

	Athena	AthSimulation	AthGeneration	AnalysisBase	AthAnalysis	DetCommon
externals
cmake
make
required tests
optional tests

Full details available on this CI monitor view. Check the JIRA CI status board for known problems
Athena: number of compilation errors 0, warnings 0
AthSimulation: number of compilation errors 0, warnings 0
AthGeneration: number of compilation errors 0, warnings 0
AnalysisBase: number of compilation errors 0, warnings 0
AthAnalysis: number of compilation errors 0, warnings 0
DetCommon: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 50321]

Changes look good from L1 point of view - approving.

Cheers, Steffen (L1)

removed review-pending-level-1 label

added review-approved label

Hi @crougier , 17 commits for these changes looks rather excessive. GitLab failed to squash commits, so could you please do that manually? See this documentation for more info. Thank you.

added review-user-action-required label

removed review-approved label

As mentioned by @tsulaia, this MR cannot be merged since the 17 commits cannot be automatically squashed by Gitlab during merging. Please squash manually and switch to rebase workflow for future MRs to avoid this problem.

Johannes (this week's RC)

added 1 commit

• e0357a81 - fix typo created during porting

Compare with previous version

WARNING: big files (>100K) are found in the changeset

144K in file InnerDetector/InDetRecTools/SiSpacePointsSeedTool_xk/src/ITkSiSpacePointsSeedMaker.cxx

This merge request affects 2 packages:

• InnerDetector/InDetRecEvent/SiSPSeededTrackFinderData
• InnerDetector/InDetRecTools/SiSpacePointsSeedTool_xk

Affected files list will not be printed in this case

Adding @goetz ,@amorley ,@gavrilen ,@sroe ,@jsandesa as watchers

added review-pending-level-1 label and removed review-user-action-required label

CI Result SUCCESS (hash e0357a81)

	Athena	AthSimulation	AthGeneration	AnalysisBase	AthAnalysis	DetCommon
externals
cmake
make
required tests
optional tests

Full details available on this CI monitor view. Check the JIRA CI status board for known problems
Athena: number of compilation errors 0, warnings 0
AthSimulation: number of compilation errors 0, warnings 0
AthGeneration: number of compilation errors 0, warnings 0
AnalysisBase: number of compilation errors 0, warnings 0
AthAnalysis: number of compilation errors 0, warnings 0
DetCommon: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 50704]

Is the thread resolved now? L1

removed review-pending-level-1 label

added review-user-action-required label

resolved all threads

resolved all threads

added review-approved label and removed review-user-action-required label

The code was approved before and just squashed. Approving.

Johannes (RC)

merged

mentioned in commit ef1776b0

added sweep:ignore label

mentioned in merge request !52555 (merged)