lpython icon indicating copy to clipboard operation
lpython copied to clipboard

Failing symbolic case example

Open anutosh491 opened this issue 1 year ago • 8 comments

What doesn't work

from lpython import S
from sympy import Symbol, log, E, Pow, exp, pi

def mmrv(e: S, x: S) -> list[S]:
    if e.args[0] != E:
        e1: S = E * pi
    else:
        list4: list[S] = [x]
        return list4

def test_mrv():
    x: S = Symbol("x")
    ans4: list[S] = mmrv(exp(log(x)), x)
    print(ans4[0])

test_mrv()
(lf) anutosh491@spbhat68:~/lpython/lpython$ lpython --enable-symengine integration_tests/test_gruntz.py 
Segmentation fault
(lf) anutosh491@spbhat68:~/lpython/lpython$ lpython --enable-symengine --backend=c integration_tests/test_gruntz.py 
Segmentation fault

anutosh491 avatar Mar 18 '24 03:03 anutosh491

What works

from lpython import S
from sympy import Symbol, log, E, Pow, exp, pi

def mmrv(e: S, x: S) -> list[S]:
    if e.args[0] != E:
        e1: S = E
    else:
        list4: list[S] = [x]
        return list4

def test_mrv():
    x: S = Symbol("x")
    ans4: list[S] = mmrv(exp(log(x)), x)
    print(ans4[0])

test_mrv()
(lf) anutosh491@spbhat68:~/lpython/lpython$ lpython --enable-symengine integration_tests/test_gruntz.py 
x
(lf) anutosh491@spbhat68:~/lpython/lpython$ lpython --enable-symengine --backend=c integration_tests/test_gruntz.py 
x

anutosh491 avatar Mar 18 '24 03:03 anutosh491

Also something similar which works

def mmrv(e: S, x: S) -> list[S]:
    if e.args[0] != E:
        a: S = E
        b: S = pi
        e1: S = a * b
    else:
        list4: list[S] = [x]
        return list4

def test_mrv():
    x: S = Symbol("x")
    ans4: list[S] = mmrv(exp(log(x)), x)
    print(ans4[0])

test_mrv()
(lf) anutosh491@spbhat68:~/lpython/lpython$ lpython --enable-symengine --backend=c integration_tests/test_gruntz.py 
x
(lf) anutosh491@spbhat68:~/lpython/lpython$ lpython --enable-symengine integration_tests/test_gruntz.py 
x

This hints that it might be something related to declaring or freeing of variables.

anutosh491 avatar Mar 18 '24 03:03 anutosh491

Smallest failing example

from lpython import S
from sympy import Symbol, E

def mmrv(e: S) -> S:
    if False:
        print(E)
    else:
        return e

def test_mrv():
    x: S = Symbol("x")
    ans: S = mmrv(x)
    print(ans)

test_mrv()

The C code for this tells me that we are trying to free the variable (using basic_free_stack) even before creating a stack for it (using basic_new_stack), Hence the error I think

void mmrv(void* e, void* _lpython_return_variable)
{
    int64_t _stack0;
    void* stack0;
    if (false) {
        _stack0 = 0;
        stack0 = NULL;
        stack0 = (void*) &_stack0;
        basic_new_stack(stack0);
        basic_const_E(stack0);
        printf("%s\n", basic_str(stack0));
    } else {
        basic_assign(_lpython_return_variable, e);
        basic_free_stack(stack0);
        return;
    }
    basic_free_stack(stack0);
}

anutosh491 avatar Mar 18 '24 03:03 anutosh491

One approach that comes to my mind is the following, i) We can maybe introduce a function in symengine's cwrapper.h (and it's corresponding implementation in cwrapper.cpp)

int basic_has_allocated_stack(basic s);

int basic_has_allocated_stack(basic s) {
    return s != nullptr;
}

And then check everytime before freeing if it was allocated on the stack beforehand. Not sure if this is go how we should proceed.

ii) Or maybe something like basic_free_stack_if_allocated that first checks if the basic variable is NULL and if it's not then frees it !

iii) And even if we don't plan to make any changes through SymEngine just adding a statement like this

        if (stack0 != NULL) {
            basic_free_stack(stack0);
        }
        

instead of just basic_free_stack(stack0) would work

anutosh491 avatar Mar 18 '24 06:03 anutosh491

The bug is in the else branch:

    } else {
        basic_assign(_lpython_return_variable, e);
        basic_free_stack(stack0);
        return;
    }

Where we are freeing the stack0 that has not been allocated, so it fails.

certik avatar Mar 20 '24 16:03 certik

I can see two approaches forward:

  1. We initialize stack0 as NULL at the beginning, and we only free it if it is not null
  2. We always call basic_new_stack(stack0) every time we create a local variable stack0 and then we always deallocate it.
  3. Keep the stack0 inside the "if" statement where it is needed (the "then" branch of the if statement only)

Here is the Python code:

def mmrv(e: S) -> S:
    if False:
        print(E)
    else:
        return e

Here is how 1. would be applied to the this code:

void mmrv(void* e, void* _lpython_return_variable)
{
    int64_t _stack0;
    void* stack0;
    stack0 = NULL;
    if (false) {
        _stack0 = 0;
        stack0 = (void*) &_stack0;
        basic_new_stack(stack0);
        basic_const_E(stack0);
        printf("%s\n", basic_str(stack0));
    } else {
        basic_assign(_lpython_return_variable, e);
        basic_free_stack_if_not_null(stack0);
        return;
    }
    basic_free_stack_if_not_null(stack0);
}

Here is how 2. would be applied to the above:

void mmrv(void* e, void* _lpython_return_variable)
{
    int64_t _stack0;
    void* stack0;
    _stack0 = 0;
    stack0 = NULL;
    stack0 = (void*) &_stack0;
    basic_new_stack(stack0);
    if (false) {
        basic_const_E(stack0);
        printf("%s\n", basic_str(stack0));
    } else {
        basic_assign(_lpython_return_variable, e);
        basic_free_stack(stack0);
        return;
    }
    basic_free_stack(stack0);
}

Here is how 3. would be applied to the above:

void mmrv(void* e, void* _lpython_return_variable)
{
    if (false) {
        int64_t _stack0;
        void* stack0;
        _stack0 = 0;
        stack0 = NULL;
        stack0 = (void*) &_stack0;
        basic_new_stack(stack0);
        basic_const_E(stack0);
        printf("%s\n", basic_str(stack0));
        basic_free_stack(stack0);
    } else {
        basic_assign(_lpython_return_variable, e);
        return;
    }
}

certik avatar Mar 20 '24 16:03 certik

I think we can go for the first approach, I've raise a pr to symengine (https://github.com/symengine/symengine/pull/2011) for the same.

anutosh491 avatar Mar 27 '24 07:03 anutosh491

Let's go with 1. for now. Let's write a function in the symbolic pass that will insert the equivalent of:

    if (stack0 != NULL) {
        basic_free_stack(stack0);
    }

into ASR.

certik avatar Apr 03 '24 16:04 certik