Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enabling c_sym backend for symbolics_14 #2467

Closed

Conversation

anutosh491
Copy link
Collaborator

Now that #2454 has been merged (fixing issues related to the c_sym backend) , I think symbolics_14 can be supported through it.

@anutosh491
Copy link
Collaborator Author

I do my few doubts though. Consider symbolics_14

def mmrv(x: S) -> list[S]:
    l1: list[S] = [x]
    return l1

def test_mrv1():
    x: S = Symbol("x")
    ans: list[S] = mmrv(x)
    element_1: S = ans[0]
    print(element_1)
    assert element_1 == x

test_mrv1()

Now after the subroutine_from_function pass this changes to

def mmrv(x: S, r: Out[list[S]]) -> None:
    l1: list[S]
    l1 = [x]
    r = l1

def test_mrv1():
    x: S = Symbol("x")
    ans: list[S]
    temp: list[S]
    mmrv(x, temp)
    ans = temp
    ele1: S = ans[0]
    print(ele1)

test_mrv1()

Here my observation says

  1. That the element for creating the list constant is being passed as an argument to mmrv
  2. Hence if we look at the c code produced for this, we have constname0.data[0] = x; rather than basic_assign(constname0.data[0], x) which I think we would prefer

And hence this ends up working with both llvm_sym and c_sym backends

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

@anutosh491
Copy link
Collaborator Author

But if we check the example out of #2450 or (symbolics_15 too in that case)
If we are not passing the list element as an argument and use a local variable inside the mmrv function , something like

def mmrv(r: Out[list[S]]) -> None:
    x: S = pi
    l1: list[S]
    l1 = [x]

Even for this case we would end up with a constname0.data[0] = x; but for this case (where the element is defined locally ) this doesn't work anymore and we end up getting a Segmentation Fault

Hence although I have enabled the c backend for symbolics_14, I think we should discuss why this works and if it is intended to work !

@anutosh491
Copy link
Collaborator Author

We should also discuss how assignment of a symbolic list has been done in symbolics_11.py
It is pretty similar to the above raised concern.

@certik
Copy link
Contributor

certik commented Jan 31, 2024

To answer your questions, it seems the main bug is #2469, so let's fix it. Then we can worry about the rest if there are any other bugs.

@anutosh491
Copy link
Collaborator Author

The c_sym backend was correctly supported for symbolics_14.py through #2477, hence this PR can now be closed.

@anutosh491 anutosh491 closed this Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants