Added Support for Symbolic func attribute

[anutosh491]

This Pr tries to add support for the func attribute. I've used a transform the following python program into this respective C code

def main0():
    x: S = pi
    y: S = Symbol("y")
    z: S = y + x
    y = y**x
    print(y.func)

def main0():
    _x: i64 = i64(0)
    x: CPtr = empty_c_void_p()
    p_c_pointer(pointer(_x, i64), x)
    basic_new_stack(x)
    basic_const_pi(x)
    _y: i64 = i64(0)
    y: CPtr = empty_c_void_p()
    p_c_pointer(pointer(_y, i64), y)
    basic_new_stack(y)
    symbol_set(y, "y")
    basic_pow(y, y, x)
    temp_int : i32 = basic_get_type(y)
    temp_str : str = ""
    if temp_int == 17:
        temp_str = "Pow"
    if temp_int == 15:
        temp_str = "Mul"
    if temp_int == 16:
        temp_str = "Add"
    print(temp_str)

main0()

So essentially we do the following

1. Intorduce temp_str, temp_int variables and introduce the respective symbols in the symbol table
2. Frame if statements where we compare temp_int against integers and update temp_str accordingly

[anutosh491]

Hence something like the following works now

(lf) anutosh491@spbhat68:~/lpython/lpython$ cat examples/expr2.py 
from lpython import S
from sympy import Symbol, pi

def main0():
    x: S = pi
    y: S = Symbol("y")
    z: S = y + x
    y = y**x
    print(y.func)
    print(z.func)
    assert (y.func == "Pow")
    assert (z.func == "Add")

if __name__ == "__main__":
   main0()
(lf) anutosh491@spbhat68:~/lpython/lpython$ lpython --enable-symengine examples/expr2.py 
Pow
Add

[anutosh491]

The implementation is a surface level implementation right now, we can surely improve or optimize it here and there.
cc @Thirumalai-Shaktivel if you see anything where I could improve upon or maybe a different implementation idea as a whole to address the func attribute you could let me know.

[certik]

Instead of:

    temp_str : str = ""
    if temp_int == 17:
        temp_str = "Pow"
    if temp_int == 15:
        temp_str = "Mul"
    if temp_int == 16:
        temp_str = "Add"

Let's send a patch to SymEngine to add a function symengine_type_to_str(17) -> "Pow", then you can just call it here.

[certik]

I think the .func attribute can't be of a type string. Rather, it must be able to compare the Pow and Add symbols, and we can assume that we know the actual value of the RHS at compile time, so that we can always directly translate it to our ASR.

[anutosh491]

Yeah returning a string was hackish here and not intended.
If only we had subtypes for in the ASR for SymbolicExpression (like SymbolicPowExpression etc) to further classify a SymbolicExpression it would have been great.

ttype
    = Integer(int kind)
    ....
    | SymbolicExpression()
    | FunctionType(ttype* arg_types, ttype? return_var_type,
        abi abi, deftype deftype, string? bindc_name, bool elemental,
        bool pure, bool module, bool inline, bool static,
        symbol* restrictions, bool is_restriction)

What are your thoughts here ? Which ttype should the func attribute return ?

[anutosh491]

Let's send a patch to SymEngine to add a function symengine_type_to_str(17) -> "Pow", then you can just call it here.

I think so we already have basic_get_class_from_id(https://github.com/symengine/symengine/blob/2b575b9be9bb499d866dc3e411e6368ca0d1bb42/symengine/cwrapper.h#L155) which can take in an Integer and return the class (as a string). Let me try using this !

[anutosh491]

I've made the implementation much easier through the latest commit.
Now something like

z: str = y.func
print(z)

Is transformed into the following through the pass

z: str = basic_get_class_from_id(basic_get_type(y))
print(z)

The only question that remains is what should be the return type of the func attribute. I'm a bit confused here. Once I have some clarity on this, I would finish work on this !

[certik]

I can see the following ways forward:

1. Adding types to ASR for symbolic expressions, such as SymbolicPowExpression. We might end up doing that, but I think it's a little bit early for this, I would do this later once we gain more experience. Adding types I think is a big change since now every place that accepts a type must learn how to handle the new symbolic types. First I would do some more minimal solution, and iterate. Once we actually use this in limits or other bigger codes, we will gain more experience how to design this well.
2. Add SymbolicPowQ, SymbolicLogQ, etc. nodes, that accept an expression and return true/false. Internally you implement them using SymEngine, one way or the other. I don't see any problem here, except that there might be a long list of these nodes (but there is going to be a long list somewhere anyway for 1. as well, as well as for 3. and 4.).
3. Add SymbolicIsAQ node that accepts an expression and a SymEngine Type ID integer
4. Make func return an integer, the SymEngine Type ID integer.

Related issue: #2332

This requires a proper design, but for now let's get something going that unblocks us, and then we'll iteratively improve things. I suspect the fundamental issue here is that we are querying the type at runtime, so it's a runtime type. While in LPython so far all types are compile time.

I like the approach 2. the most. You can think of all those ASR IntrinsicFunctions like SymbolicPowQ as "runtime types", without introducing any integer ID, so this design does not close any door to a more sophisticated design. At the frontend side, you can easily map (x**2).func == Pow to SymbolicPowQ(x**2) in ASR, and in the backend, just implement SymbolicPowQ like you already did (I believe) by hardwiring the SymEngine ID number (for now), and later we'll initialize the Type ID numbers at the program start (at runtime), or even require to link against exactly the same symengine that was available at compile time, in which case we can simply hardwire the numbers for the given symengine version. You can use some macros to quickly add support for all SymEngine types, one macro call (one source code line) per type, so this will be easy to maintain.

Do you see any problem with this approach?

[anutosh491]

I like the approach 3.

You mean approach 2 (did you type 3 by mistake ?) where we implement SymbolicPowQ etc ?

[anutosh491]

I'll just write down my understanding here so that anyone can correct me if I haven't understood things correctly.
Well if I go through the def mrv functions in gruntz.py, I see that there are type checks and some other queries like the following

1) e.is_Pow, e.is_Mul, e.is_Add
2) isinstance(e, log), isinstance(e, exp)
3) others such as e.has(x), e.base == S.Exp1, e.is_infinite

This makes me realize that we only need to focus on the query aspects of things here ( only having a bool output either True or False)
So essentially we don't need to think about cases like assigning or printing right now correct ?

def main0():
    x: S = pi
    y: S = Symbol("y")
    y = y**x
    print(y.func)

Hence we do not have to think about the return type of the func attribute right now (Am I correct ?) !
Out of the 3 cases , we just need to focus on the 3rd case and also the query aspect of func.

1) type : some_type = e.func
2) print(e.func)
3) e.func == Pow

If my thinking is correct we should be able to map/transform e.func to use SymbolicPow somehow.

[certik]

You mean approach 2 (did you type 3 by mistake ?) where we implement SymbolicPowQ etc ?

Yes, I fixed it. I like approach 2 the most.

[certik]

Yes, what you wrote is correct. In the end, did you mean SymbolicPowQ? Here is how I would tackle each of the cases you mentioned:

1) e.is_Pow, e.is_Mul, e.is_Add

I would use SymbolicPowQ(e), SymbolicMulQ(e), SymbolicAddQ(e).

2) isinstance(e, log), isinstance(e, exp)

I would use SymbolicLogQ(e), SymbolicExpQ(e).

3) others such as e.has(x)

I would use SymbolicHasSymbolQ(e, x) (already implemented)

3) e.base == S.Exp1

I would use SymbolicExp1Q(SymbolicArgs(e, 0)) (and 1 for "e.exp"). Here you can start with SymbolicExp1Q. Normally I would name it just SymbolicEQ but that looks like some kind of an "equation" or "equality", so better to use SymbolicExp1Q for now.

3) e.is_infinite

I would use SymbolicInfinityQ(e).

[Thirumalai-Shaktivel]

2. Add SymbolicPowQ, SymbolicLogQ, etc. nodes,

You meant to add in IntrinsicScalarFunction, right?
If so, then should we move symbolics items from IntrinisicScalarFunction to dedicated enum, say: Symbolics?

[certik]

You meant to add in IntrinsicScalarFunction, right?
If so, then should we move symbolics items from IntrinisicScalarFunction to dedicated enum, say: Symbolics?

Yes, those SymbolicPowQ etc. are part of IntrinsicScalarFunction. For now I would keep them there. Once we can compile the full limit.py and have more experience, we can figure out if a better design can be done. Right now I think it is not clear which approach is better, so we go with the "minimal design", which this one is, as it doesn't require to modify ASR at all.

[certik]

There are several ways to tackle it, but right now we have not figured out the most cleanest way.

To move forward, let's implement it like this:

• In AST->ASR, in visit_Compare(AST::Compare_t ...) we add a line of the kind: "is the LHS a .func called on a symbolic type"?
• If true: check that the right hand side is exactly "Pow" (and other types) and return SymbolicPowQ, otherwise give a nice error message
• If false: handle the compare node as usual

The alternative implementation:

• You descent all the way to .func, and you set some internal flag (as well as some internal type) that the whole Compare node must be replaced
• In visit_Compare check this flag and if true, replace Compare with SymbolicPowQ.

[anutosh491]

You descent all the way to .func, and you set some internal flag (as well as some internal type) that the whole Compare node must be replaced

I've tried addressing the func attribute through this method in the latest commit. I think using an internal flag would be a suitable approach here. So now we can do something like the following

(lf) anutosh491@spbhat68:~/lpython/lpython$ cat examples/expr2.py 

def main0():
    x: S = pi
    y: S = Symbol("y")
    y = y + x
    z1: bool = y.func == Pow
    z2: bool = y.func == Add
    print(z1)
    print(z2)

(lf) anutosh491@spbhat68:~/lpython/lpython$ lpython --enable-symengine examples/expr2.py 
False
True

And for something that is yet to be implemented

def main0():
    x: S = pi
    y: S = Symbol("y")
    y = y + x
    z1: bool = y.func == Log
    print(z1)
    
 (lf) anutosh491@spbhat68:~/lpython/lpython$ lpython --enable-symengine examples/expr2.py 
semantic error: Log symbolic type not supported yet
 --> examples/expr2.py:8:16
  |
8 |     z1: bool = y.func == Log
  |                ^^^^^^^^^^^^^ 

[anutosh491]

The latest commit also adds some simple tests for the same in symbolics_02.py

[certik]

In addition to this, let's also add tests like this:

if z.func == Add:
    assert True
else:
    assert False

as well as:

assert z.func == Add

To ensure that the z.func == Add idiom works in all the situations:

• assigning to a bool variable (already tested)
• in if
• in assert

[certik]

This should still be addressed.

[certik]

Can you add a test assert z.func == Add ? I don't see it, unless I missed it.

[anutosh491]

Will remember this and add it in a subsequent pr

[certik]

Otherwise this is good.

[anutosh491]

I've addressed all changes and added relevant tests. I think this is ready. We just need to remember than we are currently hardcoding values (TypeId's) for symbolic classes like Pow (17), Mul (15) etc and we might want to replace these later using basic_get_class_from_id or something!

[certik]

I think that this is fine. Do you want to merge as is, and send improvements in subsequent PRs?

[anutosh491]

Sure . I will be addressing the isinstance(e, Exp) and isinstance(e, Log) cases in the next pr and I'll shall address anything that needs to added there.