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

fix: Fixed passing of ceil_mode key-word argument in ivy.avg_pool2d() function call #27852

Merged
merged 3 commits into from
Jan 12, 2024

Conversation

Sai-Suraj-27
Copy link
Contributor

PR Description

In the following function call, the key-word argument pad_mode is used,
https://github.com/unifyai/ivy/blob/207dec0cee120a5898749d7b3f7b984341fff313/ivy/functional/frontends/mindspore/ops/function/nn_func.py#L146-L155
From the actual definition of the ivy.avg_pool2d(), the key-word argument should be ceil_mode
https://github.com/unifyai/ivy/blob/207dec0cee120a5898749d7b3f7b984341fff313/ivy/functional/ivy/experimental/layers.py#L375-L387

Related Issue

Closes #27851

Checklist

  • Did you add a function?
  • Did you add the tests?
  • Did you run your tests and are your tests passing?
  • Did pre-commit not fail on any check?
  • Did you follow the steps we provided?

Socials

@KareemMAX
Copy link
Contributor

This function seems to be incorrectly defined, it should follow mindspore.ops.avg_pool2d signature

Copy link
Contributor

@KareemMAX KareemMAX left a comment

Choose a reason for hiding this comment

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

@Sai-Suraj-27
Copy link
Contributor Author

This function seems to be incorrectly defined, it should follow mindspore.ops.avg_pool2d signature

Hey, @KareemMAX I corrected the function def, it's arguments and default values. 👍🏻

@Sai-Suraj-27
Copy link
Contributor Author

Hey, @vedpatwardhan can you once look into this PR. Also, the frontend tests for mindspore are implemented yet or not? because I see the following. (Me and Kareem discussed about it but we are unsure). Thank you.
https://github.com/unifyai/ivy/blob/b024de17148728f658c30db8116547ed8d4daa58/ivy_tests/test_ivy/test_frontends/test_mindspore/test_ops/test_function/test_mindspore_nn_func.py#L203-L204

@vedpatwardhan
Copy link
Contributor

Hey, @vedpatwardhan can you once look into this PR. Also, the frontend tests for mindspore are implemented yet or not? because I see the following. (Me and Kareem discussed about it but we are unsure). Thank you.

https://github.com/unifyai/ivy/blob/b024de17148728f658c30db8116547ed8d4daa58/ivy_tests/test_ivy/test_frontends/test_mindspore/test_ops/test_function/test_mindspore_nn_func.py#L203-L204

The changes look good to me, feel free to merge. Yeah the tests aren't working for the mindspore frontend at the moment 😄

@KareemMAX KareemMAX merged commit cb2e589 into ivy-llc:main Jan 12, 2024
109 of 141 checks passed
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.

Unexpected keyword argument pad_mode in function call ivy.avg_pool2d()
4 participants