lcompilers
Added support for i32, i64, f32 and f64's sqrt and cbrt
Till now, LPython's math module has sqrt() and cbrt() functions that work only for f64 types. In this PR, I have added support for i32, i64, f32 and f64 datatypes, along with an integration test.
@Kishan-Ved I had done some research on the sqrt() method and found that raising a number to the power (1/2) is not computationally efficient for calculating square roots of large numbers . There are other efficient methods like the Newton-Raphson method.
I request you to implement them. 🙂
@Kishan-Ved I had did some research on the
sqrt()method and found that raising a number to the power (1/2) is not computationally efficient for calculating square roots of large numbers . There are other efficient methods like the Newton-Raphson method.I request you to implement them. 🙂
Yes @kmr-srbh , I've heard about them. My only concern is that these methods (like Newton-Raphson) give approximate solutions. We need the error to be lesser than 1e-12 from the actual square root (at least as per the current rigour of the tests). If we implement these methods, we will need a large number of iterations to get this precision (and there might be cases when we never achieve it). Also, python's floating point precision might cause some errors. Nevertheless, I'll see if these can be implemented, thanks! :)
You are right @Kishan-Ved. Newton-Raphson method was just an example. It is not ideal for our use case. In fact it resorts to infinite loops for some numbers!
I am excited for what you implement! :)
I think these sqrt() and cbrt() functions should ideally be implemented as a part of the intrinsic registry functions.
But for the moment, the current approach in this PR also seems fine to me.
The changes seem fine to me for the short term. For the long term we need these to be implemented as intrinsic registry functions.
We need to discuss/decide on the return values for
sqrtandcbrtin case ofi32andf32arguments.
We have sqrt implemented in the intrinsic function registry. How do we redirect the control to it?
In ASR we represent sqrt as IntrinsicScalarFunction, then it is redirected to IntrinsicFunctionSqrt for float types in instantiate_Sqrt in the intrinsic_function_registry.h file. As LLVM provides llvm.f32.sqrt intrinsic support, we compute the output using that.
Now, I think we have to do the same for cbrt as well.