Skip to content

feat: implement symbolicEqual function#2424

Merged
josdejong merged 1 commit into
josdejong:developfrom
gwhitney:symbolic_equal
Mar 1, 2022
Merged

feat: implement symbolicEqual function#2424
josdejong merged 1 commit into
josdejong:developfrom
gwhitney:symbolic_equal

Conversation

@gwhitney
Copy link
Copy Markdown
Collaborator

Note for this to work in a broad variety of contexts, has to also allow
identical expressions to cancel regardless of whether subtraction is
always defined; but this seems safe in general, that x-x is 0 even if x
does not generally have an additive inverse (for example, when working
in the positive context).

Resolves #1260.

  Note for this to work in a broad variety of contexts, has to also allow
  identical expressions to cancel regardless of whether subtraction is
  always defined; but this seems safe in general, that x-x is 0 even if x
  does not generally have an additive inverse (for example, when working
  in the positive context).

  Resolves josdejong#1260.
@joshhansen
Copy link
Copy Markdown

I just tried this out (not this code specifically, but the same technique) and am pleasantly surprised. It's an elegant approach.

The weakness seems to be the dependence on simplify, which (IMHO) is not aggressive enough. (I know there's ongoing work on that front.)

But for my current use case (multiplying polynomial factors of a polynomial to see if the original polynomial is yielded) this seems like it will be a great addition. And as simplify grows in capability, the likelihood of false negatives will further diminish.

@gwhitney
Copy link
Copy Markdown
Collaborator Author

Yes, absolutely, we want simplify to take advantage of as many opportunities as it can. If you have specific examples of expressions that are not being simplified, and what you would like them to simplify to, please please post them as issues. It is a very active area for mathjs right now and the more instances of useful simplifications we have the better mathjs can be in this area. Thanks!

@joshhansen
Copy link
Copy Markdown

@gwhitney I left a comment on #2388 that shows a few cases which are not simplifying for me, even with your new code on that branch.

@gwhitney
Copy link
Copy Markdown
Collaborator Author

I added them to the tests on that branch, and they seemed to pass OK. Thanks!

@josdejong
Copy link
Copy Markdown
Owner

Thanks Glen, looks good 👍

I'll solve the merge conflict and merge this PR now.

@josdejong josdejong merged commit c983008 into josdejong:develop Mar 1, 2022
@josdejong
Copy link
Copy Markdown
Owner

Published in v10.3.0

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.

Node simplify or deepEqual ordering

3 participants