xreplace #1320
xreplace #1320
Conversation
symengine/subs.h
Outdated
| : BaseVisitor<SubsVisitor, XReplaceVisitor>(xreplace_dict_) | ||
| { | ||
| } | ||
|
|
||
| void bvisit(const Derivative &x) |
There was a problem hiding this comment.
You should move this to xreplace. See sympy's implementation
symengine/subs.h
Outdated
| @@ -252,8 +363,8 @@ class SubsVisitor : public BaseVisitor<SubsVisitor> | |||
| break; | |||
| } | |||
| } else { | |||
| result_ | |||
| = make_rcp<const Subs>(x.rcp_from_this(), subs_dict_); | |||
| result_ = make_rcp<const Subs>(x.rcp_from_this(), | |||
There was a problem hiding this comment.
@isuruf, If Derivative is moved to xreplace, does returning an instance of Subs class on a call to xreplace make sense?
There was a problem hiding this comment.
Ah. Fine then
ranjithkumar007
force-pushed
the
xreplace
branch
from
df3b4b0
to
f5457a5
Compare
|
ping @isuruf |
symengine/subs.h
Outdated
| protected: | ||
| RCP<const Basic> result_; | ||
| const map_basic_basic &subs_dict_; | ||
| const map_basic_basic &xreplace_dict_; |
There was a problem hiding this comment.
Let's leave subs_dict_ as it was.
symengine/subs.h
Outdated
| and is_a<Pow>(*((*xreplace_dict_.begin()).first)) | ||
| and not is_a<Add>( | ||
| *down_cast<const Pow &>(*(*xreplace_dict_.begin()).first) | ||
| .get_exp())) { |
There was a problem hiding this comment.
What happens to (2**(x+y+z)).subs({2**(x+z): 1})?
There was a problem hiding this comment.
2**(x+y+z), stays as it is.
There was a problem hiding this comment.
same in SymPy.
symengine/subs.h
Outdated
| { | ||
| } | ||
|
|
||
| void bvisit(const Pow &x) |
There was a problem hiding this comment.
Only Pow? Isn't Add and Mul also needed?
There was a problem hiding this comment.
on second thought, Add and Mul are not performing any such simplifications. So, It might be better to keep it as it is.
|
ping @srajangarg |
|
can you please have a look @Sumith1896 ? |
cc @isuruf.