Overlap of behavior between simplifyCore and simplifyConstant?

[gwhitney]

Currently, the operations of simplifyCore and simplifyConstant overlap with each other, but neither subsumes the other:

expression	simplifyConstant	simplifyCore
x+0	x+0	x
1+2	3	3
[1,2]+[3,4]	[4,6]	[1,2]+[3,4]

Basically, simplifyConstant doesn't touch any non-constant subexpressions, whereas simplifyCore does do some straightforward one-pass simplifications like 1*n -> n. On the other hand, simplifyConstant tries essentially to fully evaluate all subexpressions that involve only constants (not certain that it's yet 100% comprehensive, but that seems to be the aspiration), whereas simplifyCore just collapses a few node types all of whose children are just numeric constants.

This seems a bit confusing to me, and not the most "clean." It seems to me that either simplifyCore should do everything that simplifyConstant does plus a bit more, for the most obvious straightforward things like additive and multiplicative identities; or simplifyCore should not touch constant-only subexpressions and leave them to simplifyConstant to handle, focusing just on the things that simplifyConstant can't do.

Do you agree? If so, please just let me know which organization of the behavior of simplifyCore and simplifyConstant you prefer, and I will file a PR establishing and documenting that relationship between them. One final issue is that simplifyCore is exported as a mathJS function but simplifyConstant is not. If we go with simplifyCore subsuming simplifyConstant, that seems fine; but if they are changed to be disjoint, or if the current status quo is otherwise left, then I would recommend exporting simplifyConstant so that mathjs clients can obtain the behavior of fully simplifying constant expressions.

Looking forward to your feedback.

[josdejong]

I understand the confusion. If simplifyConstant does not have a clear meaningful purpose it could be merged into simplifyCore. Or maybe some other abstraction makes more sense. (on the other hand, if simplifyCore grows too large and does do too much it can also become complicated again).

Feel free to play around and see if a refactoring works out nicely 👍 . By now I think you're more authoritative in this piece of code than me (I have never worked on the code myself 😉), so please let me know what you think would be a better separation of functionality.

[gwhitney]

I think that simplifyConstant is a very natural operation, partly because of its role in the demo symbolic computation #2470, and is worth having in its own right. I don't see any need to partially replicate its behavior in simplifyCore. But there is a need for a function that performs a number of "non-controversial" single-pass simplifications, that can be used as one rule in the full-blown simplify. So when I have a chance I will issue a PR that separates their behavior and exports them both as mathjs typed functions.

[josdejong]

Thanks 👍