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

Support for MOD, PMOD and PYMOD for decimal32/64/128 #10179

Merged
merged 13 commits into from
Feb 3, 2022

Conversation

codereport
Copy link
Contributor

@codereport codereport commented Feb 1, 2022

Resolves libcudf MOD and PMOD portion of #7132

To Do:

  • MOD
  • PMOD
  • PYMOD
  • Unit tests
  • Different scaled tests

@codereport codereport added feature request New feature or request 2 - In Progress Currently a work in progress libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change labels Feb 1, 2022
@codereport codereport self-assigned this Feb 1, 2022
@codecov
Copy link

codecov bot commented Feb 1, 2022

Codecov Report

❗ No coverage uploaded for pull request base (branch-22.04@c25d35b). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##             branch-22.04   #10179   +/-   ##
===============================================
  Coverage                ?   10.48%           
===============================================
  Files                   ?      122           
  Lines                   ?    20496           
  Branches                ?        0           
===============================================
  Hits                    ?     2148           
  Misses                  ?    18348           
  Partials                ?        0           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c25d35b...d523944. Read the comment docs.

@codereport codereport added 3 - Ready for Review Ready for review by team 4 - Needs Review Waiting for reviewer to review or respond and removed 2 - In Progress Currently a work in progress labels Feb 1, 2022
@codereport codereport marked this pull request as ready for review February 1, 2022 18:26
@codereport codereport requested a review from a team as a code owner February 1, 2022 18:26
Copy link
Contributor

@hyperbolic2346 hyperbolic2346 left a comment

Choose a reason for hiding this comment

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

Copyrights need updating here.

Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

Comments attached.

cpp/include/cudf/fixed_point/fixed_point.hpp Outdated Show resolved Hide resolved
cpp/include/cudf/fixed_point/fixed_point.hpp Outdated Show resolved Hide resolved
cpp/include/cudf/fixed_point/fixed_point.hpp Show resolved Hide resolved
cpp/include/cudf/fixed_point/fixed_point.hpp Outdated Show resolved Hide resolved
cpp/src/binaryop/binaryop.cpp Outdated Show resolved Hide resolved
cpp/src/binaryop/compiled/operation.cuh Show resolved Hide resolved
cpp/src/binaryop/binaryop.cpp Outdated Show resolved Hide resolved
cpp/tests/binaryop/binop-compiled-fixed_point-test.cpp Outdated Show resolved Hide resolved
@codereport codereport changed the title Support for PMOD and MOD for decimal32/64/128 Support for MOD, PMOD and PYMOD for decimal32/64/128 Feb 1, 2022
@codereport codereport requested a review from bdice February 2, 2022 17:51
Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

A few comments on is_modulo_binop.

cpp/src/binaryop/binaryop.cpp Outdated Show resolved Hide resolved
cpp/src/binaryop/binaryop.cpp Outdated Show resolved Hide resolved
cpp/src/binaryop/binaryop.cpp Outdated Show resolved Hide resolved
cpp/src/binaryop/binaryop.cpp Outdated Show resolved Hide resolved
@codereport codereport requested a review from bdice February 2, 2022 22:22
Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

LGTM!

cpp/src/binaryop/compiled/operation.cuh Show resolved Hide resolved
rapids-bot bot pushed a commit that referenced this pull request Feb 3, 2022
As pointed out by @bdice in #10179, the docs are backwards for some of the binary operations on `fixed_point`. This PR fixes them and removes the `<br>` HTML tag as well.

Authors:
  - Conor Hoekstra (https://github.com/codereport)

Approvers:
  - Bradley Dice (https://github.com/bdice)
  - David Wendt (https://github.com/davidwendt)
  - MithunR (https://github.com/mythrocks)

URL: #10198
@codereport
Copy link
Contributor Author

@gpucibot merge

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 4 - Needs Review Waiting for reviewer to review or respond feature request New feature or request libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants