Implements partial substitution of Mul objects

[eeshan9815]

Fixes #1359
Added 11 test cases which demonstrate the partial substitution of Mul in mixed-variable terms as well.
ping @isuruf

[isuruf-bot]

Hi,

I've run clang-format and found that the code needs formatting.
Here's a commit that fixes this. isuruf-bot@47616f0

To use the commit you can do

curl -o format.diff https://github.com/isuruf-bot/symengine/commit/47616f0b062d66db394f188d633a3fa70444a07c.diff
git apply format.diff

[ShikharJ]

@eeshan9815 You can run ./bin/test_format_local.sh before committing the changes. That would automatically format the code.

[eeshan9815]

@ShikharJ Okay, thanks!

[eeshan9815]

@isuruf can you please review the PR?

[isuruf]

This is handled in the first part. Why check again?

[eeshan9815]

@isuruf
The first part is the earlier implementation that only looks for exact matches, and not partial matches which should be handled using subs.
That part is retained only for the fast execution speeds in case of exact matches as it doesn't involve many heavy computations.

[isuruf]

Here's some test cases to handle,

In [14]: t = 2*x* y * z * w

In [15]: t.subs({x*y: 2,z*w:4})
Out[15]: 16

In [16]: t.subs({x*y: 2,z:1})
Out[16]: 4*w

In [17]: t.subs({2*x*y: 5,2*z*w:3})
Out[17]: 3*x*y

In [18]: t.subs({2*x*y: 5,z*w:3})
Out[18]: 15

[eeshan9815]

In [17]: t.subs({2*x*y: 5,2*z*w:3})
Out[17]: 3*x*y

How should it be decided whether 3*x*y is returned or 5*z*w?

[isuruf]

How should it be decided whether 3xy is returned or 5zw?

Doesn't matter, use the first match.

[eeshan9815]

Here's some test cases to handle,
In [14]: t = 2*x* y * z * w

In [15]: t.subs({x*y: 2,z*w:4})
Out[15]: 16

In [16]: t.subs({x*y: 2,z:1})
Out[16]: 4*w

In [17]: t.subs({2*x*y: 5,2*z*w:3})
Out[17]: 5*z*w

In [18]: t.subs({2*x*y: 5,z*w:3})
Out[18]: 15

I have added the above test cases to test_subs.cpp and all test cases are passing.
([17] matches to 5\*z\*w, not 3\*x\*y)

[eeshan9815]

ping @isuruf

[isuruf]

Can you explain this part? Why is sub1 being a Symbol important? I thought the issue was that sub1 being a Mul isn't handled correctly.

[eeshan9815]

Partial substitution can happen for Symbol objects as well. For example, if the expression is x**2*y and sub1 is x, there is still partial substitution possible.
The first part only looks for exact matches.

Follow up question, should substitution happen repeatedly until not possible? For instance, if the expression is x**2*y**3 and x*y is to be replaced by z, should the result be x*y**2*z or y*z**2?

[isuruf]

Hmm, (x**2*y).subs({x: z}) didn't work before?

[eeshan9815]

It did, but I've made the non fast_exec implementation complete. Should I remove these redundant parts?

[eeshan9815]

ping @isuruf

[isuruf]

If it's redundant, remove them

[isuruf]

@eeshan9815, sorry about the delay. Can you remove redundant code here?

[certik]

Does this PR affect performance?

[eeshan9815]

@certik It is slower than the earlier implementation because that only naively looked at the expression for exact matches.
I have optimised the partial substitution part of the function by comparing and checking if the exponent of the expression to be substituted is smaller than that of the original expression rather than checking for exact matches of all the possibilities.
Also, if an exact match is found, it will be run as fast as the earlier implementation.

[eeshan9815]

@isuruf is there more work to be done before merging?

[eeshan9815]

ping @isuruf