Adding Support for symbolics in the list data structure #2368

anutosh491 · 2023-10-07T09:24:34Z

This is an attempt to allow the list data structure to handle symbolics.
So suppose we have something like

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

def main0():
    l1: list[S] = [pi]
    
main0()

Through the pass the following takes place

First we run through the list and then for each element (either a variable like x or an intrinsic function like pi, we do whatever's required like introducing the placeholder , changing type from S to CPtr)
Then we change the type of the list itself.
So after the pass we have

def main0():
    _stack0: i64 = i64(0)
    stack0: CPtr = empty_c_void_p()
    p_c_pointer(pointer(_stack0, i64), stack0)
    basic_new_stack(stack0)
    basic_const_pi(stack0)
    l1: list[CPtr] = [stack0]

main0()

anutosh491 · 2023-10-07T09:29:41Z

But I am not sure if this is what we want. As in we also need to think about supporting the print operation (which is something I would like to address next). Considering the print operation we would like to have this

def main0():
    _stack0: i64 = i64(0)
    stack0: CPtr = empty_c_void_p()
    p_c_pointer(pointer(_stack0, i64), stack0)
    basic_new_stack(stack0)
    basic_const_pi(stack0)
    l1: list[str] = [basic_str(stack0)]
    print(l1)

main0()

OR if we don't want to lose the CPtr based list we need to do introduce a temporary list

def main0():
    _stack0: i64 = i64(0)
    stack0: CPtr = empty_c_void_p()
    p_c_pointer(pointer(_stack0, i64), stack0)
    basic_new_stack(stack0)
    basic_const_pi(stack0)
    l1: list[CPtr] = [stack0]
    _l1: list[str] = [basic_str(stack0)]
    print(l1)

main0()

anutosh491 · 2023-10-07T11:22:22Z

I'm also a bit confused about transforming the following through the pass. Consider the following program where we are extracting elements out of the list

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

def main0():
    l1: list[S] = [pi]
    ele1: S = l1[0]
    print(ele1)

I would expect something like this to do the job.

from lpython import ccall, CPtr, p_c_pointer, pointer, i64, empty_c_void_p
import os

@ccall(header="symengine/cwrapper.h", c_shared_lib="symengine", c_shared_lib_path=f"{os.environ['CONDA_PREFIX']}/lib")
def basic_new_stack(x: CPtr) -> None:
    pass

@ccall(header="symengine/cwrapper.h", c_shared_lib="symengine", c_shared_lib_path=f"{os.environ['CONDA_PREFIX']}/lib")
def basic_const_pi(x: CPtr) -> None:
    pass

@ccall(header="symengine/cwrapper.h", c_shared_lib="symengine", c_shared_lib_path=f"{os.environ['CONDA_PREFIX']}/lib")
def basic_assign(x: CPtr, y: CPtr) -> None:
    pass

@ccall(header="symengine/cwrapper.h", c_shared_lib="symengine", c_shared_lib_path=f"{os.environ['CONDA_PREFIX']}/lib")
def basic_str(x: CPtr) -> str:
    pass

def main0():
    _stack0: i64 = i64(0)
    stack0: CPtr = empty_c_void_p()
    p_c_pointer(pointer(_stack0, i64), stack0)
    basic_new_stack(stack0)
    basic_const_pi(stack0)
    l1: list[CPtr] = [stack0]
    _ele1: i64 = i64(0)
    ele1: CPtr = empty_c_void_p()
    p_c_pointer(pointer(_ele1, i64), ele1)
    basic_new_stack(ele1)
    basic_assign(ele1, l1[0])
    print(basic_str(ele1))

main0()

But I get a segmentation fault when I try resolving this

(lf) anutosh491@spbhat68:~/lpython/lpython$ lpython --enable-symengine integration_tests/symbolics_08.py
Segmentation fault

certik · 2023-10-07T12:07:01Z

That should work. Try a simpler example first:

def main0():
    l1: list[CPtr] = [??]
    CPtr: S = l1[0]

And put something instead of ??, to ensure that list[CPtr] works.

anutosh491 · 2023-10-08T11:04:43Z

CPtr: S = l1[0]

I didn't catch this line correctly. As in l1[0] would be of type CPtr right ?
I still fail to get this working correctly. I thought once we extract the first element from the list

ele1: S = l1[0]

We just need to declare it as we have been doing and everything should work fine

    _ele1: i64 = i64(0)
    ele1: CPtr = empty_c_void_p()
    p_c_pointer(pointer(_ele1, i64), ele1)
    basic_new_stack(ele1)
    basic_assign(ele1, l1[0])

anutosh491 · 2023-10-08T11:10:21Z

Also I observe something weird. Not sure we need to change anything here but this went unnoticed because everything worked perfectly. Consider the following

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)
    y = x
    print(basic_str(y))
    
(lf) anutosh491@spbhat68:~/lpython/lpython$ lpython --enable-symengine integration_tests/symbolics_08.py
pi

The work around we introduced allows us to assign a basic variable from right to left like we would do by default. But symengine documentation stresses that we should only use basic_assign(y, x) to assign one variable to another (https://github.com/symengine/symengine/blob/2b575b9be9bb499d866dc3e411e6368ca0d1bb42/symengine/cwrapper.h#L80)

Even that works perfectly though

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)
    basic_assign(y, x)
    print(basic_str(y))
    
(lf) anutosh491@spbhat68:~/lpython/lpython$ lpython --enable-symengine integration_tests/symbolics_08.py
pi

This is something fundamental as per symengine but I think so everything worked correctly hence we missed implemented basic_assign in our tests !

certik · 2023-10-08T12:47:37Z

Yes, y = x is incorrect. It must be basic_assign(y, x). Otherwise reference counting is wrong, which might explain the segfault you are seeing.

certik · 2023-10-08T12:48:40Z

Yes, I wrote it wrong. It should be:

def main0():
    l1: list[CPtr] = [??]
    x: CPtr = l1[0]

anutosh491 · 2023-10-09T02:20:47Z

I was just trying to figure to why do I see a segmentation fault when I realized that the following doesn't work

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)
    l1: list[CPtr] = [x]
    _y: i64 = i64(0)
    y: CPtr = empty_c_void_p()
    p_c_pointer(pointer(_y, i64), y)
    basic_new_stack(y)
    basic_assign(y, l1[0])
    print(basic_str(y))

main0()(lf) anutosh491@spbhat68:~/lpython/lpython$ lpython --enable-symengine integration_tests/symbolics_08.py
Segmentation fault

But the following does work

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)
    l1: list[CPtr] = []
    l1.append(x)
    _y: i64 = i64(0)
    y: CPtr = empty_c_void_p()
    p_c_pointer(pointer(_y, i64), y)
    basic_new_stack(y)
    basic_assign(y, l1[0])
    print(basic_str(y))

main0()(lf) anutosh491@spbhat68:~/lpython/lpython$ lpython --enable-symengine integration_tests/symbolics_08.py
pi

Not sure why this would be the case. Trying to think through it.

certik · 2023-10-09T07:00:23Z

There might be something funny with the initializers for lists. You have to analyze the ASR and then the generated LLVM or C code.

anutosh491 · 2023-10-10T05:39:11Z

Well the C code generated works perfectly for both cases. I pasted the above cases in symbolics_08 and symbolics_07 and get the following through the c backend.

(lf) anutosh491@spbhat68:~/lpython/lpython$ lpython --enable-symengine --backend=c integration_tes
ts/symbolics_07.py
pi

(lf) anutosh491@spbhat68:~/lpython/lpython$ lpython --enable-symengine --backend=c integration_tes
ts/symbolics_08.py
pi

I think the issue here lies on with the llvm backend !

certik · 2023-10-10T05:42:06Z

Great. We just have to debug it.

anutosh491 · 2023-10-10T06:28:48Z

The latest commit added support for declaring and extracting elements out of a list that is symbolic in nature

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

def main0():
    l1: list[S] = [pi, Symbol("x")]
    y1: S = l1[0]
    y2: S = l1[1]
    print(y1)
    print(y2)
    

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

Though currently the llvm backend gives us a segmentation fault, at the ASR level we are ready!

I haven't thought of how but I'll see if we could support printing of the list next.

certik · 2023-10-10T07:17:21Z

Perfect! Just create a test and only enable the C backend, not LLVM yet. Then we can merge this.

anutosh491 · 2023-10-10T07:26:13Z

I've added the tests, I think so the CI would pass.

certik

I think this looks great, thanks!

certik · 2023-10-10T11:33:50Z

Use the lpython symbolics_08.py --enable-symengine --show-llvm > ok.ll and lpython symbolics_08.py --enable-symengine --show-llvm > fail.ll, to get ok.ll that works and fail.ll that fails. Then compare the two to see the difference, and the bug is there.

The other advice: try to create a smaller example that still segfaults. It will make it easier to debug.

Adding Support for symbolics in the list data structure

5af8e0e

anutosh491 added the symbolic label Oct 7, 2023

anutosh491 mentioned this pull request Oct 9, 2023

Supporting assignment through basic_assign #2378

Merged

Merge branch 'main' into list_support

3e7830c

added support for extracting elements

15f5445

Added tests

1657b32

certik approved these changes Oct 10, 2023

View reviewed changes

certik merged commit 9b5f52f into lcompilers:main Oct 10, 2023
13 checks passed

anutosh491 deleted the list_support branch October 10, 2023 10:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding Support for symbolics in the list data structure #2368

Adding Support for symbolics in the list data structure #2368

anutosh491 commented Oct 7, 2023

anutosh491 commented Oct 7, 2023

anutosh491 commented Oct 7, 2023

certik commented Oct 7, 2023

anutosh491 commented Oct 8, 2023

anutosh491 commented Oct 8, 2023

certik commented Oct 8, 2023 •

edited

Loading

certik commented Oct 8, 2023

anutosh491 commented Oct 9, 2023 •

edited

Loading

certik commented Oct 9, 2023 •

edited

Loading

anutosh491 commented Oct 10, 2023

certik commented Oct 10, 2023

anutosh491 commented Oct 10, 2023 •

edited

Loading

certik commented Oct 10, 2023

anutosh491 commented Oct 10, 2023

certik left a comment

certik commented Oct 10, 2023 •

edited

Loading

Adding Support for symbolics in the list data structure #2368

Adding Support for symbolics in the list data structure #2368

Conversation

anutosh491 commented Oct 7, 2023

anutosh491 commented Oct 7, 2023

anutosh491 commented Oct 7, 2023

certik commented Oct 7, 2023

anutosh491 commented Oct 8, 2023

anutosh491 commented Oct 8, 2023

certik commented Oct 8, 2023 • edited Loading

certik commented Oct 8, 2023

anutosh491 commented Oct 9, 2023 • edited Loading

certik commented Oct 9, 2023 • edited Loading

anutosh491 commented Oct 10, 2023

certik commented Oct 10, 2023

anutosh491 commented Oct 10, 2023 • edited Loading

certik commented Oct 10, 2023

anutosh491 commented Oct 10, 2023

certik left a comment

Choose a reason for hiding this comment

certik commented Oct 10, 2023 • edited Loading

certik commented Oct 8, 2023 •

edited

Loading

anutosh491 commented Oct 9, 2023 •

edited

Loading

certik commented Oct 9, 2023 •

edited

Loading

anutosh491 commented Oct 10, 2023 •

edited

Loading

certik commented Oct 10, 2023 •

edited

Loading