Skip to content

Implementing Symbolic ASR pass #2239

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

Closed
wants to merge 14 commits into from
Closed

Conversation

anutosh491
Copy link
Collaborator

The pass should be responsible for converting the ASR of Program 1 to Prorgam 2
Program 1

from lpython import S
from sympy import pi

def main0():
    x: S = pi
    print(x)

if __name__ == "__main__":
   main0()

Program 2

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_str(x: CPtr) -> str:
    pass

def main0():
    y: i64 = i64(0)
    x: CPtr = empty_c_void_p()
    p_c_pointer(pointer(y, i64), x)
    basic_new_stack(x)
    basic_const_pi(x)
    print(basic_str(x))

if __name__ == "__main__":
   main0()

@anutosh491 anutosh491 marked this pull request as draft July 31, 2023 10:53
@anutosh491
Copy link
Collaborator Author

Most of this PR was reviewed in #2200 but there was it seems there are some issues on the branch , hence I've created a duplicate branch of the same .

@Thirumalai-Shaktivel
Copy link
Collaborator

Awesome, Looks great!

Test the following and see if it works as expected.

from lpython import S
from sympy import pi

def main0():
    x: S = pi
    y: S = pi
    print(x)

if __name__ == "__main__":
   main0()

@anutosh491
Copy link
Collaborator Author

Hello @Thirumalai-Shaktivel , I have implemented the visit_Print function in the latest commit to transform the Print node from block 1 to block 2

# block 1
                                    (Print
                                        ()
                                        [(Var 3 x)]
                                        ()
                                        ()
                                    )
                                    
 # block 2
 (Print
                                        ()
                                        [(FunctionCall
                                            2 basic_str
                                            2 basic_str
                                            [((Var 3 x))]
                                            (Character 1 -2 ())
                                            ()
                                            ()
                                        )]
                                        ()
                                        ()
                                    )

I now get the required result for the program you've mentioned above through the llvm and the c backend

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

@Thirumalai-Shaktivel
Copy link
Collaborator

Beautiful! Great progress!

@anutosh491
Copy link
Collaborator Author

The symbol_set function would look something like

@ccall(header="symengine/cwrapper.h")
def symbol_set(x: CPtr, s: str) -> None:
    pass

Hence I think I have the correct implementation in place for it . But for the following program , I am not able to get the pass working unlike the pi assignment case .

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

def main0():
    x: S = Symbol("x")
    print(x)

if __name__ == "__main__":
   main0()
(lf) anutosh491@spbhat68:~/lpython/lpython$ lpython --show-asr --pass=symbolic examples/expr2.py
ASR verify pass error: ASR verify: Function symbol_set doesn't depend on basic_new_stack but is found in its dependency list.
Internal Compiler Error: Unhandled exception
Traceback (most recent call last):
  Binary file "/home/anutosh491/lpython/lpython/src/bin/lpython", in _start()
  Binary file "/lib/x86_64-linux-gnu/libc.so.6", in __libc_start_main()
  File "/home/anutosh491/lpython/lpython/src/bin/lpython.cpp", line 1797, in ??
    return emit_asr(arg_file, lpython_pass_manager, runtime_library_dir,
  File "/home/anutosh491/lpython/lpython/src/bin/lpython.cpp", line 252, in ??
    pass_manager.apply_passes(al, asr, pass_options, diagnostics);
  File "/home/anutosh491/lpython/lpython/src/libasr/pass/pass_manager.h", line 280, in LCompilers::PassManager::apply_passes(Allocator&, LCompilers::ASR::TranslationUnit_t*, LCompilers::PassOptions&, LCompilers::diag::Diagnostics&)
    apply_passes(al, asr, _user_defined_passes, pass_options,
LCompilersException: Verify failed in the pass: symbolic

It says Function symbol_set doesn't depend on basic_new_stack but is found in its dependency list. but I don't see it being added to the dependency list manually by me !

@anutosh491
Copy link
Collaborator Author

I've now added functionality for all unary and binary functions that have been implemented through these Prs
#1964 , #2077 & #2094 .
Hence from the pass now we can achieve something like this both through the C and the LLVM backend.

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

def main0():
    x: S = Symbol('x')
    y: S = sin(x)
    z: S = diff(y, x)
    print(y)
    print(z)

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

@anutosh491
Copy link
Collaborator Author

I've added the symbolic integer through casting in my latest commit to achieve the following functionality

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

def main0():
    x: S = S(20)
    y: S = S(40)
    z: S = x * y
    print(z)

if __name__ == "__main__":
   main0()

(lf) anutosh491@spbhat68:~/lpython/lpython$ lpython --enable-symengine examples/expr2.py
800

I think we could stop at this point , review this and implement other things in the subsequent PRs. @Thirumalai-Shaktivel

Copy link
Collaborator

@Thirumalai-Shaktivel Thirumalai-Shaktivel left a comment

Choose a reason for hiding this comment

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

LGTM!

@Thirumalai-Shaktivel Thirumalai-Shaktivel marked this pull request as ready for review August 3, 2023 14:27
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 this is fine.

How long will it take to re-enable at least half of the other tests?

@certik certik changed the title WIP: Implementing Symbolic ASR pass Implementing Symbolic ASR pass Aug 4, 2023
@anutosh491
Copy link
Collaborator Author

anutosh491 commented Aug 4, 2023

I think it should all be done within a weeks time !
Only two things need to be supported

  1. Generating temporary variables using the symengine queue and using that whenever required (chaining of operators etc)
  2. Supporting assert statements

Actually 2 won't be challenging once 1 is done .
For eg we can do everything in this program (symbolics_01.py) except the last line (assert statement) . Now that is because the right hand side is pi + y for which we would need 1 . pi + y would be stored in queue0 , which would be a variable/symbol introduced in the symbol table and assert would just compare z and queue0.

from sympy import Symbol, pi
from lpython import S

def main0():
    x: S = Symbol('x')
    y: S = Symbol('y')
    x = pi
    z: S = x + y
    print(z)
    assert(z == pi + y)

@certik
Copy link
Contributor

certik commented Aug 4, 2023

This PR will not conflict, since it just adds a pass. So let's keep it open. Let's not add any more commits to this PR.

To implement 1. above, just open a new PR, that builds upon this one. Implement 1. We'll see how many tests will start working.

What I don't want to happen is the current capability to break while we are building this. Only once most of the tests pass, I am happy to do the switch.

@anutosh491
Copy link
Collaborator Author

anutosh491 commented Aug 4, 2023

Yes I think we should proceed like above . Thanks for the review .

@certik
Copy link
Contributor

certik commented Aug 8, 2023

Merged as part of #2255.

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

Successfully merging this pull request may close these issues.

4 participants