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

Fix integer comparison bugs in device CKF #694

Merged

Conversation

stephenswat
Copy link
Member

This commit fixes a few bugs in the device CKF to do with the integral representation of the previous step number. Firstly, the previous step of step 0 (e.g. some sort of sentinel value) was being inconsistently defined as either the maximum value of a signed int or an unsigned int, and this was stored in an integer, leading to unexpected behaviour. Furthermore, the track building kernel was not correctly checking for the sentinel value, but rather looking for values smaller than 0.

@stephenswat stephenswat added bug Something isn't working shared Changes related to shared code labels Sep 2, 2024
@stephenswat stephenswat force-pushed the fix/ckf_numerical_limits branch from dcb8712 to 20a8b3c Compare September 2, 2024 15:52
@stephenswat
Copy link
Member Author

@krasznaa updated as you requested.

Copy link
Contributor

@beomki-yeo beomki-yeo left a comment

Choose a reason for hiding this comment

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

Ah, finding the bug at the track building kernel is a good catch

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 stephenswat force-pushed the fix/ckf_numerical_limits branch from 20a8b3c to 6d203b4 Compare September 2, 2024 19:04
This commit fixes a few bugs in the device CKF to do with the integral
representation of the previous step number. Firstly, the previous step
of step 0 (e.g. some sort of sentinel value) was being inconsistently
defined as either the maximum value of a signed int or an unsigned int,
and this was stored in an integer, leading to unexpected behaviour.
Furthermore, the track building kernel was not correctly checking for
the sentinel value, but rather looking for values smaller than 0.
@stephenswat stephenswat merged commit bb6ff01 into acts-project:main Sep 2, 2024
20 of 24 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