Skip to content

Implement take_along_axis function per Python Array API #1778

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

Merged
merged 10 commits into from
Aug 6, 2024

Conversation

oleksandr-pavlyk
Copy link
Contributor

@oleksandr-pavlyk oleksandr-pavlyk commented Aug 1, 2024

The function is planned for Python Array API 2024.12 specification.

  • 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?

Copy link

github-actions bot commented Aug 1, 2024

Deleted rendered PR docs from intelpython.github.com/dpctl, latest should be updated shortly. 🤞

@coveralls
Copy link
Collaborator

coveralls commented Aug 1, 2024

Coverage Status

coverage: 87.927% (+0.2%) from 87.739%
when pulling d07de32 on take-along-axis
into d79dae1 on master.

Copy link

github-actions bot commented Aug 1, 2024

Array API standard conformance tests for dpctl=0.18.0dev0=py310ha798474_205 ran successfully.
Passed: 894
Failed: 1
Skipped: 119

Copy link

github-actions bot commented Aug 1, 2024

Array API standard conformance tests for dpctl=0.18.0dev0=py310ha798474_208 ran successfully.
Passed: 894
Failed: 1
Skipped: 119

Copy link

github-actions bot commented Aug 1, 2024

Array API standard conformance tests for dpctl=0.18.0dev0=py310ha798474_209 ran successfully.
Passed: 894
Failed: 1
Skipped: 119

Copy link

github-actions bot commented Aug 2, 2024

Array API standard conformance tests for dpctl=0.18.0dev0=py310ha798474_235 ran successfully.
Passed: 894
Failed: 1
Skipped: 119

Copy link

github-actions bot commented Aug 2, 2024

Array API standard conformance tests for dpctl=0.18.0dev0=py310ha798474_237 ran successfully.
Passed: 894
Failed: 1
Skipped: 119

Copy link

github-actions bot commented Aug 2, 2024

Array API standard conformance tests for dpctl=0.18.0dev0=py310ha798474_239 ran successfully.
Passed: 894
Failed: 1
Skipped: 119

Copy link

github-actions bot commented Aug 2, 2024

Array API standard conformance tests for dpctl=0.18.0dev0=py310ha798474_240 ran successfully.
Passed: 894
Failed: 1
Skipped: 119

@oleksandr-pavlyk oleksandr-pavlyk marked this pull request as ready for review August 2, 2024 20:12
Copy link

github-actions bot commented Aug 2, 2024

Array API standard conformance tests for dpctl=0.18.0dev0=py310ha798474_245 ran successfully.
Passed: 894
Failed: 1
Skipped: 119

1 similar comment
Copy link

github-actions bot commented Aug 2, 2024

Array API standard conformance tests for dpctl=0.18.0dev0=py310ha798474_245 ran successfully.
Passed: 894
Failed: 1
Skipped: 119

Copy link

github-actions bot commented Aug 3, 2024

Array API standard conformance tests for dpctl=0.18.0dev0=py310ha798474_246 ran successfully.
Passed: 894
Failed: 1
Skipped: 119

Copy link
Collaborator

@antonwolfy antonwolfy left a comment

Choose a reason for hiding this comment

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

I've opened a PR to reuse the new dpctl function in dpnp-1969. The existing and new dpnp tests are passed with that dpctl PR locally.

@ndgrigorian
Copy link
Collaborator

@oleksandr-pavlyk
I am going to try to take a look at this today.

One thing that I did realize, though: would it make sense to add put_along_axis as an inverse function? Array API obviously won't yet due to mutation issues, but we already have put, it seems like a good idea, maybe for a separate PR.

@oleksandr-pavlyk
Copy link
Contributor Author

One thing that I did realize, though: would it make sense to add put_along_axis as an inverse function?

Yes, makes total sense. Let's do it in a separate PR though.

Copy link
Collaborator

@ndgrigorian ndgrigorian left a comment

Choose a reason for hiding this comment

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

I've tested the branch out and it works, this LGTM.

Thank you @oleksandr-pavlyk !

@oleksandr-pavlyk oleksandr-pavlyk merged commit f492493 into master Aug 6, 2024
46 of 49 checks passed
@oleksandr-pavlyk oleksandr-pavlyk deleted the take-along-axis branch August 6, 2024 03:41
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.

4 participants