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

Added support for Out[S] #2419

Merged
merged 6 commits into from
Nov 13, 2023
Merged

Added support for Out[S] #2419

merged 6 commits into from
Nov 13, 2023

Conversation

anutosh491
Copy link
Collaborator

Thanks to the well framed subroutine_from_function pass. Now we can return symbolic objects through a cummulative result of the subroutine_from_function and symbolic pass.

@anutosh491
Copy link
Collaborator Author

I think the subroutine_from_function pass is aligned before the symbolic pass so we don't need to alter that!

@anutosh491
Copy link
Collaborator Author

So currently we can do something like

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

def func() -> S:
    print(Symbol("x"))
    return pi

def test_func():
    z: S = func()
    print(z)

test_func()
(lf) anutosh491@spbhat68:~/lpython/lpython$ lpython --enable-symengine --backend=c examples/expr2.py 
x
pi

But through the llvm backend we would still be stuck as we had discussed previously (#2411 (comment))

code generation error: asr_to_llvm: module failed verification. Error:
Call parameter type does not match function signature!
void** %_lpython_return_variable
 void*  call void @basic_const_pi(void** %_lpython_return_variable)
Call parameter type does not match function signature!
  %4 = load void*, void** %__libasr__created__var__0__func_call_res, align 8
 void**  call void @__module___main___func(void* %4)

@certik
Copy link
Contributor

certik commented Nov 12, 2023

I think this is good. Can you please add a test?

Also, can you submit just the libasr change to lfortran, to ensure everything works there?

@anutosh491
Copy link
Collaborator Author

I think this is good. Can you please add a test?
Also, can you submit just the libasr change to lfortran, to ensure everything works there?

Done.

@anutosh491
Copy link
Collaborator Author

We should also discuss as to how we would address comment (#2411 (comment)) because if you think about it, the function definition for mrv would look like the following

def mrv(e: S, x: S) -> tuple[dict[S, S], S]:

Trying to replicate something like the following

>>> from sympy.series.gruntz import mrv
>>> mrv(exp(x), x)
({exp(x): _Dummy_40}, {}, _Dummy_40)

Which means we need to return a tuple with a dict inside it and dictionaries aren't currently supported through the C backend, so untill we get the llvm backend working, we might be blocked.

Copy link
Contributor

@certik certik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's fine. Can you also add a test for Out[S] explicitly? Right now it is only tested implicitly via the subroutine_from_function.

@certik certik merged commit b19dc21 into lcompilers:main Nov 13, 2023
13 checks passed
@anutosh491 anutosh491 deleted the Out branch November 13, 2023 05:45
@anutosh491
Copy link
Collaborator Author

Can you also add a test for Out[S] explicitly?

Sure.

Agent-Hellboy pushed a commit to Agent-Hellboy/lpython that referenced this pull request Mar 5, 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