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

Check in of generic reduction templates and some reductions #1399

Merged
merged 27 commits into from
Oct 3, 2023
Merged

Check in of generic reduction templates and some reductions #1399

merged 27 commits into from
Oct 3, 2023

Conversation

ndgrigorian
Copy link
Collaborator

@ndgrigorian ndgrigorian commented Sep 11, 2023 •
edited
Loading

This pull request implements the necessary meta-programming utilities for reductions with arbitrary binary operators, templates for generic reductions, as well as the new functions dpctl.tensor.max, dpctl.tensor.min, dpctl.tensor.argmax, and dpctl.tensor.argmin.

These meta-programming utilities, such as Identity, make it possible to define unique aspects of a reduction such that it may be passed to a generic functor for computation.

  • Have you provided a meaningful PR description?
  • Have you added a test, reproducer or referred to an issue with a reproducer?
  • Have you tested your changes locally for CPU and GPU devices?
  • Have you made sure that new changes do not introduce compiler warnings?
  • Have you checked performance impact of proposed changes?
  • If this PR is a work in progress, are you opening the PR as a draft?

@ndgrigorian ndgrigorian force-pushed the feature/reductions-custom-ops branch from 29fb95f to 11f4c4a Compare September 11, 2023 21:07
@github-actions
Copy link

View rendered docs @ https://intelpython.github.io/dpctl/pulls/1399/index.html

@coveralls
Copy link
Collaborator

coveralls commented Sep 11, 2023 •
edited
Loading

Coverage Status

coverage: 85.879% (+0.1%) from 85.777% when pulling 04ec23b on feature/reductions-custom-ops into caa0939 on master.

@github-actions
Copy link

Array API standard conformance tests for dpctl=0.14.6dev5=py310ha25a700_3 ran successfully.
Passed: 921
Failed: 79
Skipped: 119

@github-actions
Copy link

Array API standard conformance tests for dpctl=0.14.6dev5=py310ha25a700_3 ran successfully.
Passed: 922
Failed: 78
Skipped: 119

@github-actions
Copy link

Array API standard conformance tests for dpctl=0.14.6dev5=py310ha25a700_6 ran successfully.
Passed: 922
Failed: 78
Skipped: 119

@github-actions
Copy link

Array API standard conformance tests for dpctl=0.14.6dev5=py310ha25a700_7 ran successfully.
Passed: 921
Failed: 79
Skipped: 119

@github-actions
Copy link

Array API standard conformance tests for dpctl=0.14.6dev5=py310ha25a700_8 ran successfully.
Passed: 924
Failed: 76
Skipped: 119

@ndgrigorian ndgrigorian force-pushed the feature/reductions-custom-ops branch from 9feca2d to 42d20e4 Compare September 15, 2023 07:44
@github-actions
Copy link

Array API standard conformance tests for dpctl=0.14.6dev5=py310ha25a700_9 ran successfully.
Passed: 924
Failed: 76
Skipped: 119

1 similar comment
@github-actions
Copy link

Array API standard conformance tests for dpctl=0.14.6dev5=py310ha25a700_9 ran successfully.
Passed: 924
Failed: 76
Skipped: 119

@ndgrigorian ndgrigorian force-pushed the feature/reductions-custom-ops branch from 42d20e4 to 316b88a Compare September 19, 2023 01:29
@github-actions
Copy link

Array API standard conformance tests for dpctl=0.15.0rc1=py310ha25a700_9 ran successfully.
Passed: 929
Failed: 71
Skipped: 119

@github-actions
Copy link

Array API standard conformance tests for dpctl=0.15.0rc1=py310ha25a700_10 ran successfully.
Passed: 930
Failed: 70
Skipped: 119

@github-actions
Copy link

Array API standard conformance tests for dpctl=0.15.0rc1=py310ha25a700_11 ran successfully.
Passed: 930
Failed: 70
Skipped: 119

@ndgrigorian ndgrigorian force-pushed the feature/reductions-custom-ops branch from 198785a to 4bb76a7 Compare September 22, 2023 04:00
@github-actions
Copy link

Array API standard conformance tests for dpctl=0.15.0rc2=py310ha25a700_11 ran successfully.
Passed: 930
Failed: 70
Skipped: 119

@ndgrigorian ndgrigorian force-pushed the feature/reductions-custom-ops branch 2 times, most recently from 6d10437 to d1de259 Compare September 27, 2023 05:26
@github-actions
Copy link

Array API standard conformance tests for dpctl=0.15.0rc3=py310ha25a700_13 ran successfully.
Passed: 930
Failed: 70
Skipped: 119

1 similar comment
@github-actions
Copy link

Array API standard conformance tests for dpctl=0.15.0rc3=py310ha25a700_13 ran successfully.
Passed: 930
Failed: 70
Skipped: 119

@ndgrigorian ndgrigorian force-pushed the feature/reductions-custom-ops branch 2 times, most recently from c3d1f62 to 1025b92 Compare September 27, 2023 17:38
@github-actions
Copy link

Array API standard conformance tests for dpctl=0.15.0rc3=py310ha25a700_15 ran successfully.
Passed: 927
Failed: 73
Skipped: 119

1 similar comment
@github-actions
Copy link

Array API standard conformance tests for dpctl=0.15.0rc3=py310ha25a700_15 ran successfully.
Passed: 927
Failed: 73
Skipped: 119

@github-actions
Copy link

Array API standard conformance tests for dpctl=0.15.0rc3=py310ha25a700_15 ran successfully.
Passed: 930
Failed: 70
Skipped: 119

@ndgrigorian ndgrigorian marked this pull request as ready for review September 28, 2023 01:47
@github-actions
Copy link

Array API standard conformance tests for dpctl=0.15.0=py310ha25a700_19 ran successfully.
Passed: 930
Failed: 70
Skipped: 119

@github-actions
Copy link

Array API standard conformance tests for dpctl=0.15.0=py310ha25a700_22 ran successfully.
Passed: 930
Failed: 70
Skipped: 119

ndgrigorian and others added 10 commits September 29, 2023 13:53
@ndgrigorian
Argmin and argmax now handle identities correctly
97efe7a 
Adds a test for this behavior

Fixed a typo in argmin and argmax causing shared local memory variant to be used for more types than expected
@ndgrigorian
Replaced std::min with idx_reduction_op_
7aef816
@ndgrigorian
reductions now well-behaved for size-zero arrays
6c3abcc 
- comparison and search reductions will throw an error in this case
- slips in change to align sum signature with array API spec
@ndgrigorian
removed unnecessary copies in reduction templates
a00ac58
@ndgrigorian
Refactors sum to use generic reduction templates
2468d8a
@ndgrigorian
Sum now uses a generic Python API
78829e7
@ndgrigorian
Docstrings added for argmax, argmin, max, and min
f01991b
@ndgrigorian
Small reduction clean-ups
8597300 
Removed unnecessary copies in custom_reduce_over_group

Sequential reduction now casts before calling operator (makes behavior explicit rather than implicit)
@oleksandr-pavlyk @ndgrigorian
Added test for argmin with keepdims=True
2c18667
@ndgrigorian
Added a test for raised errors in reductions
24b54d7 
Also removed unused `_usm_types` in `test_tensor_sum`
@ndgrigorian ndgrigorian force-pushed the feature/reductions-custom-ops branch from 1cda64c to 24b54d7 Compare September 29, 2023 20:53
@github-actions
Copy link

Array API standard conformance tests for dpctl=0.15.1dev0=py310ha25a700_19 ran successfully.
Passed: 930
Failed: 70
Skipped: 119

1 similar comment
@github-actions
Copy link

Array API standard conformance tests for dpctl=0.15.1dev0=py310ha25a700_19 ran successfully.
Passed: 930
Failed: 70
Skipped: 119

@github-actions
Copy link

github-actions bot commented Oct 1, 2023

Array API standard conformance tests for dpctl=0.15.1dev0=py310ha25a700_20 ran successfully.
Passed: 930
Failed: 70
Skipped: 119

@oleksandr-pavlyk
Added missing include, Identity to use has_known_identity
478b30c 
Implementation of Identity trait should call sycl::known_identity
if trait sycl::has_known_identity is a true_type.

Added IsMultiplies, and identity value for it, since sycl::known_identity
for multiplies is only defined for real-valued types.
@oleksandr-pavlyk
Adding functor factories for product over axis
0598416
@oleksandr-pavlyk oleksandr-pavlyk mentioned this pull request Oct 1, 2023
Merged
6 tasks
oleksandr-pavlyk and others added 6 commits October 1, 2023 08:21
@oleksandr-pavlyk
Added Python API for _prod_over_axis
ca0ff64
@oleksandr-pavlyk
Common reduction template takes functions to test if atomics are appl…
ee46ae1 
…icable

Passing these function pointers around allows to turn atomic off altogether
if desired.

Use custom trait to check if reduce_over_groups can be used. This allows to
work-around bug, or switch to custom code for reduction over group if desired.

Such custom trait type works around issue with incorrect result returned from
sycl::reduce_over_group for sycl::multiplies operator for 64-bit integral types.
@oleksandr-pavlyk
Defined dpctl.tensor.prod
1d9b7ce 
Also tweaked docstring for sum.
@oleksandr-pavlyk
Added tests for dpt.prod, removed uses of numpy
8890d21
@ndgrigorian
Corrected prod docstring
60a8ad7 
Small tweaks to sum, min, and max docstrings
@oleksandr-pavlyk
Merge pull request #1426 from IntelPython/implement-product
04ec23b 
Implement product over axis
oleksandr-pavlyk
oleksandr-pavlyk approved these changes Oct 3, 2023
View reviewed changes
Copy link
Collaborator

@oleksandr-pavlyk oleksandr-pavlyk left a comment

Choose a reason for hiding this comment

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

I think this PR is ready to go in. Subsequent development can be done in new PRs. Thank you @ndgrigorian !

@github-actions
Copy link

github-actions bot commented Oct 3, 2023

Array API standard conformance tests for dpctl=0.15.1dev0=py310ha25a700_28 ran successfully.
Passed: 935
Failed: 65
Skipped: 119

@ndgrigorian ndgrigorian merged commit 71e891c into master Oct 3, 2023
24 of 25 checks passed
@oleksandr-pavlyk oleksandr-pavlyk deleted the feature/reductions-custom-ops branch October 27, 2023 02:23
@oleksandr-pavlyk oleksandr-pavlyk mentioned this pull request Jan 26, 2024
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oleksandr-pavlyk oleksandr-pavlyk oleksandr-pavlyk approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ndgrigorian @coveralls @oleksandr-pavlyk