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

Added sqrt, atanh, atan, round and ceil functions to paddle.tensor.math #15819

Merged
merged 24 commits into from
May 29, 2023
Merged

Added sqrt, atanh, atan, round and ceil functions to paddle.tensor.math #15819

merged 24 commits into from
May 29, 2023

Conversation

KSANTHOSH200
Copy link
Contributor

@KSANTHOSH200 KSANTHOSH200 commented May 24, 2023

Close #15817
Close #15818
Close #15820
Close #15822
Close #15823

@KSANTHOSH200 KSANTHOSH200 changed the title Added sqrt and atanh functions to addle.tensor.math Added sqrt, atanh, atan functions to addle.tensor.math May 24, 2023
@KSANTHOSH200 KSANTHOSH200 changed the title Added sqrt, atanh, atan functions to addle.tensor.math Added sqrt, atanh, atan, round functions to addle.tensor.math May 24, 2023
@KSANTHOSH200 KSANTHOSH200 changed the title Added sqrt, atanh, atan, round functions to addle.tensor.math Added sqrt, atanh, atan, round and ceil functions to addle.tensor.math May 24, 2023
@KSANTHOSH200
Copy link
Contributor Author

Hi @Fayad-Alman,

Can you please review this PR?

Thanks.

@Fayad-Alman Fayad-Alman changed the title Added sqrt, atanh, atan, round and ceil functions to addle.tensor.math Added sqrt, atanh, atan, round and ceil functions to paddle.tensor.math May 24, 2023
Copy link
Contributor

@Fayad-Alman Fayad-Alman left a comment

Choose a reason for hiding this comment

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

Looks good @KSANTHOSH200! Just a few changes are needed.

Comment on lines 294 to 300
# ceil
@handle_frontend_test(
fn_tree="paddle.tensor.math.ceil",
dtype_and_x=helpers.dtype_and_values(
available_dtypes=helpers.get_dtypes("valid"),
),
)
Copy link
Contributor

Choose a reason for hiding this comment

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

@KSANTHOSH200 ciel is returning multiple errors, I believe it's related to the available_dtypes, which should be set to ("valid", full=False) instead of just ("valid")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made the change

Comment on lines 244 to 250
# atan
@handle_frontend_test(
fn_tree="paddle.tensor.math.atan",
dtype_and_x=helpers.dtype_and_values(
available_dtypes=helpers.get_dtypes("valid"),
),
)
Copy link
Contributor

Choose a reason for hiding this comment

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

@KSANTHOSH200 available_dtypes here should be set to "numeric" instead of "valid"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made the change.

@KSANTHOSH200
Copy link
Contributor Author

Hi @Fayad-Alman,

Made those changes.

Thanks.

@KSANTHOSH200
Copy link
Contributor Author

Hi @Fayad-Alman,

Can you please review this PR?

Thanks.

Comment on lines 294 to 299
# ceil
@handle_frontend_test(
fn_tree="paddle.tensor.math.ceil",
dtype_and_x=helpers.dtype_and_values(
available_dtypes=helpers.get_dtypes("valid", full=False),
),
Copy link
Contributor

Choose a reason for hiding this comment

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

@KSANTHOSH200 ciel seems to only take in floats; please replace the available_dtypes with "float" only

Comment on lines 244 to 250
# atan
@handle_frontend_test(
fn_tree="paddle.tensor.math.atan",
dtype_and_x=helpers.dtype_and_values(
available_dtypes=helpers.get_dtypes("numeric"),
),
)
Copy link
Contributor

Choose a reason for hiding this comment

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

@KSANTHOSH200 atan seems to take in only floats and complex numbers, so please change the available_dtypes to "float"

@Fayad-Alman
Copy link
Contributor

@KSANTHOSH200 It seems the files have conflicting changes. Please update your branch and add the changes needed.

Comment on lines 329 to 335
# round
@handle_frontend_test(
fn_tree="paddle.tensor.math.round",
dtype_and_x=helpers.dtype_and_values(
available_dtypes=helpers.get_dtypes("valid"),
),
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @KSANTHOSH200, almost done with the changes
For the function round, it doesn't take in bool dtypes, so please change available_dtypes to "float" for now
I can examine this further in the future

@Fayad-Alman
Copy link
Contributor

@KSANTHOSH200 Looks like the round function is the only faulty test. I'll be merging this for now and looking back on this later. Good work!

@Fayad-Alman Fayad-Alman merged commit 9efd0cf into ivy-llc:master May 29, 2023
@KSANTHOSH200 KSANTHOSH200 deleted the sqrt_ branch May 31, 2023 02:24
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.

ceil round atan atanh sqrt
2 participants