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

Update to work on ppc64le #7

Closed
wants to merge 1 commit into from
Closed

Update to work on ppc64le #7

wants to merge 1 commit into from

Conversation

piotrmaslanka
Copy link

@piotrmaslanka piotrmaslanka commented Jun 3, 2021

Since on POWER the input of abs() is promoted to an integer (or the result is), this condition always holds. This is a fix that hopefully makes it work on ppc64le, this was verified to work on ppc64le and returns valid results.

I've filed a concurrent pull request for OpenSfM itself, but it seems like they certainly do take their time...

Since on POWER the input of abs() is promoted to an integer, this condition always holds. This is a fix that hopefully makes it work on ppc64le.
@pierotofy
Copy link
Member

pierotofy commented Jun 15, 2021

Thanks @piotrmaslanka

You are probably not fixing the issue though, since eps (epsilon) is generally a very small floating value. The proper fix would be to change this to fabs? See https://stackoverflow.com/questions/33738509/whats-the-difference-between-abs-and-fabs

If the compiler is using the int version of abs, then you're just comparing num > 0 && num < 0, which is not the correct logic.

@pierotofy pierotofy closed this Jun 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants