Skip to content

Added support for i32, i64, f32 and f64's sqrt and cbrt #2571

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

Draft
Kishan-Ved wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Added support for i32, i64, f32 and f64's sqrt and cbrt #2571

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
87415bf
Added Support for real and integer sqrt and cbrt
Kishan-Ved Mar 3, 2024
6b11066
updated
Kishan-Ved Mar 3, 2024
f5c8b3d
updated
Kishan-Ved Mar 3, 2024
2e34731
added integration test
Kishan-Ved Mar 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
updated
  • Loading branch information
@Kishan-Ved
Kishan-Ved committed Mar 3, 2024
commit 6b11066c69a447b059c6a9cf592389faf4b1f525
8 changes: 4 additions & 4 deletions src/runtime/math.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,7 @@ def trunc(x: f32) -> i32:
@overload
def sqrt(x: f32) -> f64:
Copy link
Collaborator

@ubaidsk ubaidsk Mar 6, 2024

Choose a reason for hiding this comment

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

Should this return a f64 or an f32? Similarly for sqrt(x: i32), should it return a f64 or an f32?

It seems we might need some discussion here. I do not yet have an opinion about the type of the return value that we should return. @certik Do you have any ideas on this?

Copy link
Collaborator

@ubaidsk ubaidsk Mar 6, 2024

Choose a reason for hiding this comment

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

Similar to the return value of sqrt() that we choose/decide, we should implement/update cbrt().

Copy link
Collaborator

@Thirumalai-Shaktivel Thirumalai-Shaktivel Mar 9, 2024

Choose a reason for hiding this comment

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

I think we have to return f32, see the following the cases.

>>> import numpy as n
>>> x = n.float32(3.14)
>>> type(x)
<class 'numpy.float32'>
>>> type(n.sqrt(x))
<class 'numpy.float32'>
>>> y = 3.14
>>> type(y)
<class 'float'>
>>> import math as m
>>> type(m.sqrt(y))
<class 'float'>

If we move this implement to IntrinsicFunction in the future, this can be utilised by fortran as well, as it returns the same: https://gcc.gnu.org/onlinedocs/gfortran/SQRT.html

"""
Returns cube root of a number x
Returns sqrt root of a number x
"""
y : f64
y = f64(x)
Expand All @@ -551,14 +551,14 @@ def sqrt(x: f32) -> f64:
@overload
def sqrt(x: f64) -> f64:
"""
Returns cube root of a number x
Returns sqrt root of a number x
"""
return x**(1/2)

@overload
def sqrt(x: i32) -> f64:
"""
Returns cube root of a number x
Returns sqrt root of a number x
"""
y : f64
y = float(x)
Expand All @@ -567,7 +567,7 @@ def sqrt(x: i32) -> f64:
@overload
def sqrt(x: i64) -> f64:
"""
Returns cube root of a number x
Returns sqrt root of a number x
"""
y : f64
y = float(x)
Expand Down