Implementing Symbolic Has Query Method

[anutosh491]

Something to make note of

• Has query method from sympy can accept multiple/infinite arguments through *args and then checks for their presence through a for loop. Hence something like the following works

>>> sin(x).has(cos(x), x, sin(x**2))
True

Presence of any argument (in the above case x would return True). But currently, I don't think we are supporting any instrinsic function that takes in parameters through *args ( we obviously have intrinsic array functions but sympy's has method doesn't work on containers as it should)

>>> sin(x).has([cos(x), x, sin(x**2)])
False

After some thought, I can see that atleast for our case which is to implement the mrv set, we won't need to feed multiple arguments to the has query ( we just need e.has(x)), hence as of now I've added support through the symbolic binary macro.

[anutosh491]

But now, I don't think we have the has function already implemented in cwrapper.h file.
Do you have something in mind on how we could proceed from here ? @certik
Not sure if implementing support for the has function in the cwrapper.h is an option!

[certik]

It's implemented here: https://github.com/symengine/symengine/blob/6da98cf4527802c8c98ddc696dfd2999765cda1b/symengine/visitor.h#L129, can you send a PR to SymEngine to expose it via cwrapper.h?

[certik]

Also a test needs to be added.

[anutosh491]

It's implemented here: https://github.com/symengine/symengine/blob/6da98cf4527802c8c98ddc696dfd2999765cda1b/symengine/visitor.h#L129, can you send a PR to SymEngine to expose it via cwrapper.h?

Sure I could look into it !

[certik]

Once conda-forge/symengine-feedstock#38 is merged, then just apply the following patch:

diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml
index 50e03768c..901c609d2 100644
--- a/.github/workflows/CI.yml
+++ b/.github/workflows/CI.yml
@@ -400,7 +400,7 @@ jobs:
           create-args: >-
             python=3.10
             bison=3.4
-            symengine=0.9.0
+            symengine=0.11.1
             sympy=1.11.1
 
       - uses: hendrikmuhs/ccache-action@main

And that should allow you to use the new basic_has_symbol function from symengine, which was implemented in symengine/symengine#1978.

Also add a test for this in this PR.

[certik]

SymEngine was updated in #2346. @anutosh491 go ahead and use the new basic_has_symbol function in this PR, it should work now.

[anutosh491]

I think so all query functions in sympy return bool values but all query functions exposed through the cwrapper.h file return int outputs. I think basic_has_symbol has been framed similarly

int basic_has_symbol(const basic e, const basic s)
{
    return (int)(has_symbol(*(e->m), *(s->m)));
}

So you could let me know what we should be returning through our query methods. We should be able to tweak this if we want a bool value through the symbolic pass and replicate something like

@ccall(header="symengine/cwrapper.h", c_shared_lib="symengine", c_shared_lib_path=f"{os.environ['CONDA_PREFIX']}/lib")
def basic_has_symbol(x: CPtr, y: CPtr) -> bool:
    pass

edit1 : I'll make some changes which would make the query method return a bool value. We can change it later if we like.

[anutosh491]

Now that SymbolicHasSymbolQ returns a bool type, I would need to introduce some changes in the visit_Assert function .As of now we currently only handle SymbolicCompare nodes to support assert(sin(x).diff(x) == cos(x)) as we are just comparing symbolic expressions.
But now we would like to support asserts like assert(sin(x).has(x) == True), hence I would have to take care of LogicalCompare nodes now. I'll add the tests soon.

[anutosh491]

I think so this should be it. The Pr introduces a method called process_attributes that is used by all functions

1. visit_Print for print(sin(x).has(x))
2. visit_Assignment for a: bool = sin(x).has(x)
3. Visit_Assert for assert(sin(x).has(x) == True)

Now if we have any new attribute we would just need to register it here in the process_attributes methods and nothing else needs to be changed. Hence I don't see any repetetion through this approach.
I've also added a test for the same.

[certik]

Excellent, I will review carefully later today.

[certik]

I think that this is good, thanks for the PR!

[anutosh491]

Thanks for the reviews on this one.