Skip to content

Failing symbolic case example #2614

New issue
New issue
Open
Open
Failing symbolic case example#2614
Labels
symbolic

Description

@anutosh491
anutosh491
opened on Mar 18, 2024

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

Activity

  1. anutosh491 commented on Mar 18, 2024

    @anutosh491
    anutosh491
    on Mar 18, 2024
    CollaboratorAuthor

    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
  2. anutosh491
    added
    symbolic
    on Mar 18, 2024

  3. anutosh491 commented on Mar 18, 2024

    @anutosh491
    anutosh491
    on Mar 18, 2024
    CollaboratorAuthor

    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.

  4. anutosh491 commented on Mar 18, 2024

    @anutosh491
    anutosh491
    on Mar 18, 2024
    CollaboratorAuthor

    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);
}

  5. anutosh491 commented on Mar 18, 2024

    @anutosh491
    anutosh491
    on Mar 18, 2024
    CollaboratorAuthor

    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

  6. certik commented on Mar 20, 2024

    @certik
    certik
    on Mar 20, 2024
    Contributor

    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.

  7. certik commented on Mar 20, 2024

    @certik
    certik
    on Mar 20, 2024
    Contributor

    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;
    }
}

  8. anutosh491 commented on Mar 27, 2024

    @anutosh491
    anutosh491
    on Mar 27, 2024
    CollaboratorAuthor

    I think we can go for the first approach, I've raise a pr to symengine (symengine/symengine#2011) for the same.

  9. certik commented on Apr 3, 2024

    @certik
    certik
    on Apr 3, 2024
    Contributor

    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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    symbolic

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions