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

Minor fixes to the track building kernel #859

Merged
merged 2 commits into from
Feb 15, 2025

Conversation

stephenswat
Copy link
Member

This commit makes some minor fixes and improvements to the track building kernel. In particular, it:

  1. Fixes an error where we were comparing measurements indices to see if they were larger than the total number of measurements, but they should be check also for equality.
  2. Fail more gracefully if we cannot find any true measurements, i.e. register the failed track as a candidate for pruning.
  3. Simplify the control flow of the kernel by removing an unnecessary break statement.

@stephenswat stephenswat added the bug Something isn't working label Feb 12, 2025
@stephenswat stephenswat added the shared Changes related to shared code label Feb 12, 2025
@beomki-yeo
Copy link
Contributor

@stephenswat
Copy link
Member Author

Nice catch. Could you fix the same bugs in CPU code: https://github.com/acts-project/traccc/blob/main/core/include/traccc/finding/details/find_tracks.hpp ?

Ah yes, of course!

@stephenswat
Copy link
Member Author

There we go! 😄

Copy link
Member

@krasznaa krasznaa left a comment

Choose a reason for hiding this comment

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

👍

@stephenswat
Copy link
Member Author

Clearly this warrants some more investigation... 🤔

stephenswat added a commit to stephenswat/traccc that referenced this pull request Feb 13, 2025
While debugging the CI failures in acts-project#859, I found out that the problem
was not with my code, but rather that there is a critical failure with
the use of `std::forward` in `host_container.cpp`, which causes objects
passed to `push_back` as l-value references to be inadvertently
converted to r-value references. This commit fixes the bug in
`host_container.hpp`. The `nseed_performance_writer.cpp` contained a
much less serious mistake which I also fixed.
This commit makes some minor fixes and improvements to the track
building kernel. In particular, it:

1. Fixes an error where we were comparing measurements indices to see if
   they were _larger_ than the total number of measurements, but they
   should be check also for _equality_.
2. Fail more gracefully if we cannot find any true measurements, i.e.
   register the failed track as a candidate for pruning.
3. Simplify the control flow of the kernel by removing an unnecessary
   break statement.
@beomki-yeo
Copy link
Contributor

I will update the branch by myself and merge it once the CI passes

@beomki-yeo beomki-yeo merged commit d31f168 into acts-project:main Feb 15, 2025
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working shared Changes related to shared code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants