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.

Sign up for GitHub

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

Jump to bottom

Bug: binUtility: inside(const Vector2& lposition) might be wrong #2622

Closed
AJPfleger opened this issue Nov 2, 2023 · 1 comment · Fixed by #2666
Closed

Bug: binUtility: inside(const Vector2& lposition) might be wrong #2622

AJPfleger opened this issue Nov 2, 2023 · 1 comment · Fixed by #2666

Comments

@AJPfleger
Copy link
Contributor

AJPfleger commented Nov 2, 2023 •
edited
Loading

acts/Core/include/Acts/Utilities/BinUtility.hpp

Line 235 in 314529f

bool inside(const Vector2& lposition) const {

The Vector2-overload always returns true, but there is additional code (old version of the `Vector3-overload?) after the return.

I think this overload is not used, at least it is not tested.

The function was added in 2016 with this logic: 44cf765

How should we proceed?

  • delete overload
  • always return true
  • implement real function
@AJPfleger AJPfleger changed the title [Bug] binUtility: inside(const Vector2& lposition) might be wrong Bug: binUtility: inside(const Vector2& lposition) might be wrong Nov 2, 2023
@AJPfleger
Copy link
Contributor Author

Let's remove and find out :)

@AJPfleger AJPfleger mentioned this issue Nov 14, 2023
Merged
@kodiakhq kodiakhq bot closed this as completed in #2666 Nov 24, 2023
kodiakhq bot pushed a commit that referenced this issue Nov 24, 2023
@AJPfleger
fix: remove possible wrong function in binUtility (#2666)
a1c697c 
The code is imported and so old, that we don't know what the function should really do -> remove.

resolves #2622
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: remove possible wrong function in binUtility AJPfleger/acts
1 participant
@AJPfleger