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

Design of getargs() #2393

Closed
certik opened this issue Oct 25, 2023 · 2 comments
Closed

Design of getargs() #2393

certik opened this issue Oct 25, 2023 · 2 comments
Labels

Comments

@certik
Copy link
Contributor

certik commented Oct 25, 2023

Let's create SymbolicGetArg(expr e, int n):

--- a/src/libasr/pass/intrinsic_function_registry.h
+++ b/src/libasr/pass/intrinsic_function_registry.h
@@ -84,6 +84,7 @@ enum class IntrinsicScalarFunctions : int64_t {
     SymbolicMulQ,
     SymbolicPowQ,
     SymbolicLogQ,
+    SymbolicGetArg,
     // ...
 };

Which will take the expression e, call get_args on it, and return the n-th argument as an expression.

The SymEngine's getargs are slow anyway, since it returns std::vector<Basic> and internally SymEngine does not store the arguments in a vector, and so it must construct it (slow). Consequently, the SymbolicGetArg will be slow as well. We can start by using basic_get_args, implemented in SymEngine as:

CWRAPPER_OUTPUT_TYPE basic_get_args(const basic self, CVecBasic *args)
{
    CWRAPPER_BEGIN
    args->m = self->m->get_args();
    CWRAPPER_END
}

and then extract the n-th argument using vecbasic_get, implemented as:

CWRAPPER_OUTPUT_TYPE vecbasic_get(CVecBasic *self, size_t n, basic result)
{                     
    CWRAPPER_BEGIN                                                         
                                    
    SYMENGINE_ASSERT(n < self->m.size());
    result->m = self->m[n];                            
                                                              
    CWRAPPER_END
}                 
@certik
Copy link
Contributor Author

certik commented Oct 25, 2023

The other design is to introduce SymbolicGetArguments(expr e):

--- a/src/libasr/pass/intrinsic_function_registry.h
+++ b/src/libasr/pass/intrinsic_function_registry.h
@@ -84,6 +84,7 @@ enum class IntrinsicScalarFunctions : int64_t {
     SymbolicMulQ,
     SymbolicPowQ,
     SymbolicLogQ,
+    SymbolicGetArguments,
     // ...
 };
 

This function will return a list[S] of all the arguments at once. Then we use regular LPython mechanism for accessing a list.

This might be a better approach.

In fact this approach is equivalent to what we already designed in the past: #2332 (comment), so let's do that.

@anutosh491
Copy link
Collaborator

Closing as completed through #2399

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants