Skip to content

Copy v transform reduce out test#1856

Merged
rapids-bot[bot] merged 7 commits intorapidsai:branch-21.12from
kaatish:copy_v_transform_reduce_out_test
Oct 15, 2021
Merged

Copy v transform reduce out test#1856
rapids-bot[bot] merged 7 commits intorapidsai:branch-21.12from
kaatish:copy_v_transform_reduce_out_test

Conversation

@kaatish
Copy link
Collaborator

@kaatish kaatish commented Sep 28, 2021

  • Added testing for copy_v_transform_reduce_out_nbr primitive.
  • Fixed bug in copy_v_transform_reduce_in_out_nbr kernels.
  • Fix initial value set up in primitives.

@kaatish kaatish requested review from a team as code owners September 28, 2021 18:45
@kaatish kaatish self-assigned this Sep 28, 2021
@seunghwak seunghwak added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change and removed multi-GPU labels Sep 28, 2021
@kaatish kaatish added Graph Prims and removed improvement Improvement / enhancement to an existing function labels Sep 28, 2021
@kaatish kaatish added this to the 21.12 milestone Sep 28, 2021
@kaatish kaatish added the improvement Improvement / enhancement to an existing function label Sep 28, 2021
Copy link
Contributor

@seunghwak seunghwak left a comment

Choose a reason for hiding this comment

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

Shouldn't we have a test for in_nbr as well?

Besides this & (some minor issues about formatting), this looks great to me.

@ChuckHastings
Copy link
Collaborator

rerun tests

@codecov-commenter
Copy link

codecov-commenter commented Sep 29, 2021

Codecov Report

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

Impacted file tree graph

@@               Coverage Diff               @@
##             branch-21.12    #1856   +/-   ##
===============================================
  Coverage                ?   70.08%           
===============================================
  Files                   ?      143           
  Lines                   ?     8817           
  Branches                ?        0           
===============================================
  Hits                    ?     6179           
  Misses                  ?     2638           
  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 1313e34...219c024. Read the comment docs.

Initial value was not being set correctly. This commit fixes it.
@kaatish kaatish requested a review from seunghwak September 30, 2021 17:10
Copy link
Contributor

@seunghwak seunghwak left a comment

Choose a reason for hiding this comment

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

Looks great besides few minor cosmetic issues.

And thanks a lot for finding & fixing the bug related to handling initial values.

Copy link
Contributor

@seunghwak seunghwak left a comment

Choose a reason for hiding this comment

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

This looks great!!!

@BradReesWork
Copy link
Member

rerun tests

@ChuckHastings
Copy link
Collaborator

@rerun tests

@BradReesWork
Copy link
Member

rerun tests

1 similar comment
@ChuckHastings
Copy link
Collaborator

rerun tests

@ChuckHastings
Copy link
Collaborator

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 86a9d25 into rapidsai:branch-21.12 Oct 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Improvement / enhancement to an existing function non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants