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

Introduce ULP Based Floating Point Equality Test to Device Function #773

Merged
merged 2 commits into from
Nov 7, 2022

Conversation

isVoid
Copy link
Contributor

@isVoid isVoid commented Nov 3, 2022

Description

This PR adds float_eq_by_ulp utility to allow practical near equality for floating point computation in device code.

In GIS application, near equal is often required due to rounding error of floating point and may wrong categorization of line collinearity or point on line test.

Using this test does not incur visible performance degradation:

$ python ~/scratch/nvbench/scripts/nvbench_compare.py naive.json ulp.json 
['naive.json', 'ulp.json']
# floating_point_equivalence_benchmark

## [0] Tesla V100-SXM2-32GB

|  FloatingPointType  |  NumFloats  |   Ref Time |   Ref Noise |   Cmp Time |   Cmp Noise |      Diff |   %Diff |  Status  |
|---------------------|-------------|------------|-------------|------------|-------------|-----------|---------|----------|
|         F32         |   100000    |   2.617 us |      55.95% |   2.478 us |      53.16% | -0.139 us |  -5.30% |   PASS   |
|         F32         |   1000000   |   2.508 us |      59.48% |   2.471 us |      53.17% | -0.037 us |  -1.49% |   PASS   |
|         F32         |  10000000   |   2.509 us |      58.42% |   2.553 us |      52.45% |  0.044 us |   1.76% |   PASS   |
|         F32         |  100000000  |   2.612 us |      56.19% |   2.483 us |      55.23% | -0.129 us |  -4.94% |   PASS   |
|         F64         |   100000    |   2.501 us |      72.35% |   2.483 us |      54.59% | -0.018 us |  -0.72% |   PASS   |
|         F64         |   1000000   |   2.514 us |      60.63% |   2.564 us |      55.11% |  0.050 us |   1.98% |   PASS   |
|         F64         |  10000000   |   2.609 us |      56.87% |   2.492 us |      56.80% | -0.117 us |  -4.48% |   PASS   |
|         F64         |  100000000  |   2.519 us |      61.38% |   2.495 us |      57.26% | -0.025 us |  -0.98% |   PASS   |

Note that the run time is very small in time scale so the result is very noisy. The average time difference is very small though.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@isVoid isVoid added the 3 - Ready for Review Ready for review by team label Nov 3, 2022
@isVoid isVoid requested a review from a team as a code owner November 3, 2022 04:54
@isVoid isVoid self-assigned this Nov 3, 2022
@isVoid isVoid requested a review from a team as a code owner November 3, 2022 04:54
@isVoid isVoid requested review from trxcllnt and zhangjianting and removed request for trxcllnt November 3, 2022 04:54
@github-actions github-actions bot added the cmake Related to CMake code or build configuration label Nov 3, 2022
@isVoid isVoid removed the request for review from zhangjianting November 3, 2022 04:54
@github-actions github-actions bot added the libcuspatial Relates to the cuSpatial C++ library label Nov 3, 2022
@isVoid isVoid requested review from thomcom and harrism November 3, 2022 04:54
@isVoid isVoid added non-breaking Non-breaking change feature request New feature or request labels Nov 3, 2022
Copy link
Contributor

@thomcom thomcom left a comment

Choose a reason for hiding this comment

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

Wow, I'm really impressed.

cpp/benchmarks/floating_point_equality.cu Show resolved Hide resolved
Copy link
Member

@harrism harrism left a comment

Choose a reason for hiding this comment

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

Very useful!

cpp/include/cuspatial/detail/utility/floating_point.cuh Outdated Show resolved Hide resolved
cpp/include/cuspatial/detail/utility/floating_point.cuh Outdated Show resolved Hide resolved
cpp/tests/utility_test/test_float_equivalent.cu Outdated Show resolved Hide resolved
@isVoid isVoid requested a review from harrism November 7, 2022 18:12
@isVoid
Copy link
Contributor Author

isVoid commented Nov 7, 2022

rerun tests

@harrism
Copy link
Member

harrism commented Nov 7, 2022

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 8702bc6 into rapidsai:branch-22.12 Nov 7, 2022
rapids-bot bot pushed a commit that referenced this pull request Nov 22, 2022
#773 added a test utility that checks floating point equality based on ULP difference. However, as a blinded file checkout in #786, the line that builds tests for the floating point utility is removed by mistake. This PR adds that back.

Authors:
  - Michael Wang (https://github.com/isVoid)

Approvers:
  - Mark Harris (https://github.com/harrism)

URL: #812
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review Ready for review by team cmake Related to CMake code or build configuration feature request New feature or request libcuspatial Relates to the cuSpatial C++ library non-breaking Non-breaking change
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants