Implement Wolfram's Root builtin #806

GarkGarcia · 2020-08-25T15:13:27Z

This is a follow up to #801, an implementation of Wolfram's Root. The implementation is pretty simple, it's a wrapper for SymPy's CRootOf.

If SymPy can represent the CRootOf instance with radicals then Root resolves to the radical representation:

>> Root[x ^ 2 - 1, 1]
 = -1

Otherwise, Root resolves to itself:

>> Root[x ^ 5 + 2 x + 1, 1]
 = Root[x ^ 5 + 2 x + 1, 1]

I think a got everything working, but I was not able to test my implementation. I'm not sure how testing works in this project.

Relevant Links:

The official documentation of CRootOf: https://docs.sympy.org/latest/modules/solvers/solvers.html#algebraic-equations
An explanation of CRootOf: https://stackoverflow.com/a/36847486
Why this implementation is relevant: Solve doesn't work with certain Polynomials #801 (comment)

GarkGarcia · 2020-08-25T15:15:06Z

@weakit @sn6uv @bnjones I'd like to get your input on this.

GarkGarcia · 2020-08-25T15:25:06Z

@sn6uv @bnjones It would be great if I could make it so that Root[f, i] gets converted to CRootOf(f, i) when the to_sympy method is called. I'm not sure how to do that though.

mathics/builtin/calculus.py

weakit · 2020-08-26T07:32:19Z

@weakit @sn6uv @bnjones I'd like to get your input on this.

I'm not really familiar with how mathics works, and I'm not that good at working with big projects. I can help with bugs, but that's the most I think I'll be able to do. You'll have to ask the maintainers.

wolfv · 2020-08-26T07:35:34Z

Hi @GarkGarcia cool!
The tests in mathics are in the documentation ;)

GarkGarcia · 2020-08-26T13:35:06Z

@wolfv It looks like all tests are passing, but for some reason the Root function isn't processed at all. I thought adding a new builtin was as simple as creating a Builtin subclass and implementing apply, is there something else I need to do? Could you help me figure this out?

mathics/builtin/calculus.py

GarkGarcia · 2020-08-26T14:18:06Z

Solve[eq, x] is now working fine, but Root[f, i] isn't processed with the Root class for some reason. For example, Root[x, 1] yields Root[x, 1] instead of 0. I was able to verified that Root.apply doesn't get called by placing a random print call at the beginning of Root.apply (the text never gets print).

GarkGarcia · 2020-08-26T14:22:39Z

Solve[eq, x] is now working fine, but Root[f, i] isn't processed with the Root class for some reason. For example, Root[x, 1] yields Root[x, 1] instead of 0. I was able to verified that Root.apply doesn't get called by placing a random print call at the beginning of Root.apply (the text never gets print).

Those are the only tests that fail (besides a bunch of unrelated stuff).

wolfv · 2020-08-26T16:02:37Z

is it possible that another version of root is implemented somewhere else? Did you CTRL+F for it?

GarkGarcia · 2020-08-26T17:56:54Z

Did you CTRL+F for it?

I haven't. Just tried pressing Ctrl+F in the command line interface and nothing happened. Is this what I'm supposed to do?

wolfv · 2020-08-26T18:02:26Z

ah sorry, I meant to search the entire source code for "Root" (CTRL+F on linux is the shortcut to "search-in-page").

GarkGarcia · 2020-08-26T19:20:32Z

ah sorry, I meant to search the entire source code for "Root" (CTRL+F on linux is the shortcut to "search-in-page").

This is the output of searching for "Root" with grep

$ grep -E "Root" mathics/**.py
mathics/builtin/arithmetic.py:class CubeRoot(Builtin):
mathics/builtin/arithmetic.py:    <dt>'CubeRoot[$n$]'
mathics/builtin/arithmetic.py:    >> CubeRoot[16]
mathics/builtin/arithmetic.py:    #> CubeRoot[-5]
mathics/builtin/arithmetic.py:    #> CubeRoot[-510000]
mathics/builtin/arithmetic.py:    #> CubeRoot[-5.1]
mathics/builtin/arithmetic.py:    #> CubeRoot[b]
mathics/builtin/arithmetic.py:    #> CubeRoot[-0.5]
mathics/builtin/arithmetic.py:    #> CubeRoot[3 + 4 I]
mathics/builtin/arithmetic.py:        'CubeRoot[n_?NumericQ]': 'If[n > 0, Power[n, Divide[1, 3]], Times[-1, Power[Times[-1, n], Divide[1, 3]]]]',
mathics/builtin/arithmetic.py:        'CubeRoot[n_]': 'Power[n, Divide[1, 3]]',
mathics/builtin/arithmetic.py:        'CubeRoot[n_Complex]'
mathics/builtin/arithmetic.py:        evaluation.message('CubeRoot', 'preal', n)
mathics/builtin/calculus.py:     = RootSum[625 #1 ^ 4 + 125 #1 ^ 3 + 25 #1 ^ 2 + 5 #1 + 1&, Log[x + 5 #1] #1&] + Log[1 + x] / 5
mathics/builtin/calculus.py:class Root(Builtin):
mathics/builtin/calculus.py:    <dt>'Root[$f$, $i$]'
mathics/builtin/calculus.py:    >> Root[-1 + x ^ 2, 1]
mathics/builtin/calculus.py:    >> Root[-1 + x ^ 2, 2]
mathics/builtin/calculus.py:    Roots that can't be represented by radicals:
mathics/builtin/calculus.py:    >> Root[1 + 2 x + x ^ 5, 2]
mathics/builtin/calculus.py:     = Root[1 + 2 x + x ^ 5, 2]
mathics/builtin/calculus.py:        'Root[f, i]'
mathics/builtin/calculus.py:            r = sympy.CRootOf(f.to_sympy(), i.to_sympy() - 1)
mathics/builtin/calculus.py:            evaluation.message('Root', 'nuni', f)
mathics/builtin/calculus.py:            evaluation.message('Root', 'nint', i)
mathics/builtin/calculus.py:            evaluation.message('Root', 'iidx', i)
mathics/builtin/calculus.py:class FindRoot(Builtin):
mathics/builtin/calculus.py:    <dt>'FindRoot[$f$, {$x$, $x0$}]'
mathics/builtin/calculus.py:    <dt>'FindRoot[$lhs$ == $rhs$, {$x$, $x0$}]'
mathics/builtin/calculus.py:    'FindRoot' uses Newton\'s method, so the function of interest should have a first derivative.
mathics/builtin/calculus.py:    >> FindRoot[Cos[x], {x, 1}]
mathics/builtin/calculus.py:    >> FindRoot[Sin[x] + Exp[x],{x, 0}]
mathics/builtin/calculus.py:    >> FindRoot[Sin[x] + Exp[x] == Pi,{x, 0}]
mathics/builtin/calculus.py:    'FindRoot' has attribute 'HoldAll' and effectively uses 'Block' to localize $x$.
mathics/builtin/calculus.py:    >> FindRoot[Tan[x] + Sin[x] == Pi, {x, 1}]
mathics/builtin/calculus.py:    'FindRoot' stops after 100 iterations:
mathics/builtin/calculus.py:    >> FindRoot[x^2 + x + 1, {x, 1}]
mathics/builtin/calculus.py:    >> FindRoot[x ^ 2 + x + 1, {x, -I}]
mathics/builtin/calculus.py:    >> FindRoot[f[x] == 0, {x, 0}]
mathics/builtin/calculus.py:     = FindRoot[f[x] - 0, {x, 0}]
mathics/builtin/calculus.py:    >> FindRoot[Sin[x] == x, {x, 0}]
mathics/builtin/calculus.py:     = FindRoot[Sin[x] - x, {x, 0}]
mathics/builtin/calculus.py:    #> FindRoot[2.5==x,{x,0}]
mathics/builtin/calculus.py:        'FindRoot[lhs_ == rhs_, {x_, xs_}]': 'FindRoot[lhs - rhs, {x, xs}]',
mathics/builtin/calculus.py:        'FindRoot[f_, {x_, x0_}]'
mathics/builtin/calculus.py:            evaluation.message('FindRoot', 'snum', x0)
mathics/builtin/calculus.py:            evaluation.message('FindRoot', 'sym', x, 2)
mathics/builtin/calculus.py:                evaluation.message('FindRoot', 'dsing', x, x0)
mathics/builtin/calculus.py:                evaluation.message('FindRoot', 'nnum', x, x0)
mathics/builtin/calculus.py:            evaluation.message('FindRoot', 'maxiter')
mathics/builtin/files.py:class RootDirectory(Predefined):
mathics/builtin/files.py:    <dt>'$RootDirectory'
mathics/builtin/files.py:    >> $RootDirectory
mathics/builtin/files.py:    name = '$RootDirectory'
mathics/builtin/files.py:    #> FileNameDepth[$RootDirectory]
mathics/core/convert.py:    elif isinstance(expr, sympy.RootSum):
mathics/core/convert.py:        return Expression('RootSum', from_sympy(expr.poly),
mathics/core/convert.py:    elif isinstance(expr, sympy.CRootOf):
mathics/core/convert.py:        return Expression('Root', from_sympy(e), i + 1)

In other words, Root isn't defined anywhere else.

GarkGarcia · 2020-08-26T20:57:57Z

Just found out what the error was. As one might expect, it's one of those silly errors that are a pain to debug.

Apparently the way Mathics patterns are generated from builtin classes is via the docstring of their apply method implementations. The docstring for Root.apply was 'Root[f, i]' instead of 'Root[f_, i_]'.

… in Wolfram)

GarkGarcia · 2020-08-26T22:06:12Z

Apparently there's only one test that isn't passing (which has nothing to do with this PR). The only thing left to address is that fact that Wolfram's Root uses that slot thing (and I don't know what it is) and our Root uses regular unbound variables.

Wolfram: Root[#1 ^ 5 + 2 #1 + 1 &, 2]
This implementation: Root[x ^ 5 + 2 x + 1, 2]

I don't know how much of slot is implemented in Mathics and how could I convert a regular (univariate) equation to this format.

GarkGarcia · 2020-08-26T22:07:08Z

@weakit Perhaps you could help me understand what slot is all about.

…ces start from 1

…ics indeces start from 1" This reverts commit de25906.

suhr · 2020-08-27T01:38:48Z

https://reference.wolfram.com/language/ref/Function.html

weakit · 2020-08-27T11:27:10Z

@weakit Perhaps you could help me understand what slot is all about.

Slot is used to represent the arguments of a function.

So say you have a function f(x, y) = x² + y + 1,
In Mathematica, you could represent the function as (#1^2 + #2 + 1)&, where #1 and #2 are the first and second arguments.

if f is #1^2 + #2 + 1
f[x, y] would work out to x^2 + y + 2

Root represents the function x^5 + 2 x + 1 as #^5 + 2 # + 1.

I remember that mathics did have Slot[], but I don't remember if it worked properly. Try looking for it.

This is the way I understand it. It may be a bit off (?) so make sure to go through the documentation once.

Try going through these:
https://reference.wolfram.com/language/howto/WorkWithPureFunctions.html
https://reference.wolfram.com/language/ref/Function.html

GarkGarcia · 2020-08-27T15:30:25Z

@weakit Perhaps you could help me understand what slot is all about.

Slot is used to represent the arguments of a function.

So say you have a function f(x, y) = x² + y + 1,
In Mathematica, you could represent the function as (#1^2 + #2 + 1)&, where #1 and #2 are the first and second arguments.

if f is #1^2 + #2 + 1
f[x, y] would work out to x^2 + y + 2

Root represents the function x^5 + 2 x + 1 as #^5 + 2 # + 1.

I remember that mathics did have Slot[], but I don't remember if it worked properly. Try looking for it.

This is the way I understand it. It may be a bit off (?) so make sure to go through the documentation once.

Try going through these:
https://reference.wolfram.com/language/howto/WorkWithPureFunctions.html
https://reference.wolfram.com/language/ref/Function.html

I see. @wolfv How do I appropriately convert between a Function and a sympy.PurePoly?

suhr · 2020-08-27T15:35:21Z

The most straightforward way it to encode #1 as a variable with name _1, and so on.

… simple polynomials

GarkGarcia · 2020-08-27T16:10:42Z

The most straightforward way it to encode #1 as a variable with name _1, and so on.

Seems a bit hacky but it works well enough 😋️

Alright, I've made it so that Root takes a proper function (with slots) as it's first argument. If the implementation passes the tests we're all done in here.

suhr · 2020-08-27T16:18:34Z

You need to fix the tests though.

mathics/builtin/calculus.py

This reverts commit 7d52526.

GarkGarcia · 2020-08-27T17:30:02Z

@suhr The (#1 ...) & idea didn't work out, it caused the tests to fail because the expected results where not format in the way Mathics prints them (although both expressions represent the same value).

GarkGarcia · 2020-08-27T17:47:40Z

It looks like all relevant tests are passing. @wolfv @suhr Can we merge the PR?

wolfv · 2020-08-28T14:44:52Z

great! thanks!!

Started implementing the Root builtin

f889f06

GarkGarcia mentioned this pull request Aug 25, 2020

Solve doesn't work with certain Polynomials #801

Closed

Altered a minor detail of the implementation to make it more robust

f519406

Improved the error messages

f96f67e

GarkGarcia commented Aug 25, 2020

View reviewed changes

mathics/builtin/calculus.py Outdated Show resolved Hide resolved

GarkGarcia added the needs review label Aug 26, 2020

Fixed typo

9c53aed

wolfv reviewed Aug 26, 2020

View reviewed changes

mathics/builtin/calculus.py Show resolved Hide resolved

Fixed typo in mathics/core/convert.py

c7ae3d8

Fixed test cases in mathics/builtin/calculus.py

83484cf

Fixed test cases in mathics/builtin/calculus.py

902cc6d

GarkGarcia added 3 commits August 26, 2020 17:59

Fixed the docstring of Root.apply

ebcb9dc

Added a check for negative indices (they are allowed in SymPy but not…

14f99c2

… in Wolfram)

Added a comment to document the recently added check

91f67f9

GarkGarcia added 2 commits August 26, 2020 19:55

Adjusted the index check to account for the fact that in Mathics inde…

de25906

…ces start from 1

Revert "Adjusted the index check to account for the fact that in Math…

21cc0a9

…ics indeces start from 1" This reverts commit de25906.

Removed unnecessary files from the reposotory

3cfc40b

Made it so that Root takes proper functions (with slots) instead of…

b4f5b13

… simple polynomials

GarkGarcia added 2 commits August 27, 2020 13:35

Fixed the test cases

7808cb3

Fixed the test cases (again)

0351054

suhr reviewed Aug 27, 2020

View reviewed changes

mathics/builtin/calculus.py Outdated Show resolved Hide resolved

mathics/builtin/calculus.py Outdated Show resolved Hide resolved

mathics/builtin/calculus.py Outdated Show resolved Hide resolved

GarkGarcia added 3 commits August 27, 2020 14:12

Made the test-cases prettier

ed87bab

Made the test-cases prettier (again)

7d52526

Revert "Made the test-cases prettier (again)"

9a22567

This reverts commit 7d52526.

GarkGarcia requested a review from wolfv August 27, 2020 21:15

GarkGarcia added enhancement bug labels Aug 27, 2020

wolfv merged commit 909052e into mathics:master Aug 28, 2020

GarkGarcia deleted the root-implementation branch September 12, 2020 18:02

Uh oh!

Implement Wolfram's Root builtin #806

Implement Wolfram's Root builtin #806

Uh oh!

Conversation

GarkGarcia commented Aug 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Relevant Links:

Uh oh!

GarkGarcia commented Aug 25, 2020

Uh oh!

GarkGarcia commented Aug 25, 2020

Uh oh!

Uh oh!

weakit commented Aug 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wolfv commented Aug 26, 2020

Uh oh!

GarkGarcia commented Aug 26, 2020

Uh oh!

Uh oh!

GarkGarcia commented Aug 26, 2020

Uh oh!

GarkGarcia commented Aug 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wolfv commented Aug 26, 2020

Uh oh!

GarkGarcia commented Aug 26, 2020

Uh oh!

wolfv commented Aug 26, 2020

Uh oh!

GarkGarcia commented Aug 26, 2020

Uh oh!

GarkGarcia commented Aug 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GarkGarcia commented Aug 26, 2020

Uh oh!

GarkGarcia commented Aug 26, 2020

Uh oh!

suhr commented Aug 27, 2020

Uh oh!

weakit commented Aug 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GarkGarcia commented Aug 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

suhr commented Aug 27, 2020

Uh oh!

GarkGarcia commented Aug 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

suhr commented Aug 27, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

GarkGarcia commented Aug 27, 2020

Uh oh!

GarkGarcia commented Aug 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wolfv commented Aug 28, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

GarkGarcia commented Aug 25, 2020 •

edited

Loading

weakit commented Aug 26, 2020 •

edited

Loading

GarkGarcia commented Aug 26, 2020 •

edited

Loading

GarkGarcia commented Aug 26, 2020 •

edited

Loading

weakit commented Aug 27, 2020 •

edited

Loading

GarkGarcia commented Aug 27, 2020 •

edited

Loading

GarkGarcia commented Aug 27, 2020 •

edited

Loading

GarkGarcia commented Aug 27, 2020 •

edited

Loading