Skip to content

add kernel for max with axis #698

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 14 commits into from
Jun 22, 2021
Merged

add kernel for max with axis #698

merged 14 commits into from
Jun 22, 2021

Conversation

Rubtsowa
Copy link
Contributor

@Rubtsowa Rubtsowa commented May 6, 2021

No description provided.

@Rubtsowa Rubtsowa requested a review from shssf May 6, 2021 11:35
axis_.reserve(len(axis))
for shape_it in axis:
if shape_it < 0:
raise ValueError("DPNP dparray::__init__(): Negative values in 'shape' are not allowed")

Choose a reason for hiding this comment

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

avoid raising error on cython level. Move this, please, on python level.

@@ -301,8 +301,28 @@ def max(input, axis=None, out=None, keepdims=numpy._NoValue, initial=numpy._NoVa
"""

if not use_origin_backend(input):
isaxis = True
Copy link
Contributor

Choose a reason for hiding this comment

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

What cases you are trying to handle here? Is it a code to validate axis as input?
It would be better to write small comment in the code about it.
If it a user input validation procedure - it is better to put it in some "utils" place. Perhaps it is already where.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@shssf
DPNP dparray::init(): Negative values in 'shape' are not allowed

To avoid causing this error at the Cython level, this check has been added. The error call removal was requested in the PR above.

@shssf shssf merged commit 4411a82 into IntelPython:master Jun 22, 2021
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.

3 participants