New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Misc fixes #704

Merged
merged 5 commits into from Oct 31, 2018

Conversation

2 participants
@skirpichev
Collaborator

skirpichev commented Oct 29, 2018

No description provided.

@skirpichev skirpichev changed the title [wip] Misc fixes Misc fixes Oct 31, 2018

@skirpichev skirpichev merged commit 2b535c4 into diofant:master Oct 31, 2018

3 checks passed

codecov/patch 100% of diff hit (target 97%)
Details
codecov/project Absolute coverage decreased by -<1% but relative coverage increased by +2% compared to 22cb935
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@skirpichev skirpichev deleted the skirpichev:misc branch Oct 31, 2018

@cbm755

This comment has been minimized.

cbm755 commented on 92a7f9e Nov 21, 2018

@skirpichev SymPy has commited a fix which solves this DE in sympy/sympy#15408

pprint(dsolve(eqn))
                  ⎛ 3/4  ⎞         ⎛ 3/4  ⎞
                  ⎜2   ⋅x⎟         ⎜2   ⋅x⎟
f(x) = C₁ + C₂⋅sin⎜──────⎟ + C₃⋅cos⎜──────⎟
                  ⎝  2   ⎠         ⎝  2

Basically, call roots on the characteristic poly and if that doesn't work then fall back on RootOf. This seems like a good idea to me.


Of course, sometimes the answer can only be RootOf (such as many 5th-order DEs). Maybe there should be a test of an irrational coefficient in def test_nth_linear_constant_coeff_homogeneous_rootof().

This comment has been minimized.

Owner

skirpichev replied Nov 21, 2018

This seems like a good idea to me.

I disagree, at least if this will be done in the way it's currently implemented in SymPy. roots() may miss solutions even if it return a non-empty list.

Also, I'm not sure about the future of roots() function in the diofant. Maybe there should be some function (or method) to expand RootOf instances to radicals (if possible), like ToRadicals in Mathematica. (sympy/sympy#14884 seems to be a major obstacle on this road.) But hardly it will be roots(). I'm not sure if it worth to have a dedicated function for solving univariate polynomial equations.

Maybe there should be a test of an irrational coefficient in def test_nth_linear_constant_coeff_homogeneous_rootof().

I'm not sure I realize which test was missing in the diofant. Could you be more explicit? Are you about example from sympy/sympy#15520?

This comment has been minimized.

Owner

skirpichev replied Nov 21, 2018

BTW, diofant is able to solve problem from sympy/sympy#15520. Unfortunately, currently it only produces purely symbolic answers (i.e. RootOf's with doman=EX). Answer is not too helpful without further rewriting to something else (e.g. you can't evalf() it).

With diofant#478 branch you can have:

In [1]: eq = f(x).diff(x, 5) + sqrt(3)*f(x).diff(x) - 2*f(x)                                                                            

In [2]: dsolve(eq)                                                                                                                      
Out[2]: 
                ⎛ 5     ___         ⎞               ⎛ 5     ___         ⎞               ⎛ 5     ___         ⎞               ⎛ 5     __
        x⋅RootOf⎝x  + ╲╱ 3 ⋅x - 2, 0⎠       x⋅RootOf⎝x  + ╲╱ 3 ⋅x - 2, 1⎠       x⋅RootOf⎝x  + ╲╱ 3 ⋅x - 2, 2⎠       x⋅RootOf⎝x  + ╲╱ 3
f(x) = ℯ                             ⋅C₁ + ℯ                             ⋅C₂ + ℯ                             ⋅C₃ + ℯ                  

_         ⎞               ⎛ 5     ___         ⎞   
 ⋅x - 2, 3⎠       x⋅RootOf⎝x  + ╲╱ 3 ⋅x - 2, 4⎠   
           ⋅C₄ + ℯ                             ⋅C₅

In [3]: _2.n()                                                                                                                          
Out[3]: 
                       0.868843931790026⋅x                      x⋅(-1.00749201215938 - 0.854850813016237⋅ⅈ)                      x⋅(-1
f(x) = 2.71828182845905                   ⋅C₁ + 2.71828182845905                                           ⋅C₂ + 2.71828182845905     

.00749201215938 + 0.854850813016237⋅ⅈ)                      x⋅(0.573070046264363 - 0.995049952957158⋅ⅈ)                      x⋅(0.5730
                                      ⋅C₃ + 2.71828182845905                                           ⋅C₄ + 2.71828182845905         

70046264363 + 0.995049952957158⋅ⅈ)   
                                  ⋅C₅

Unfortunately, root isolation for polynomials over algebraic domains currently is very slow (the good news is that it was implemented with algorithms already provided by SymPy, with a tiny generalization). I'm not sure about merging mentioned work to the next release. There is a lot of room for improvements if we adopt semi-numerical approach (e.g. using mpmath's polyroots() to offer initial guesses for root isolation rectangles). But I would like to be sure - there is no important mistakes with current purely-symbolical approach to root isolation. Too slow arithmetics for algebraic numbers or something like that.
@cbm755, I appreciate your feedback on this pull request.

This comment has been minimized.

cbm755 replied Nov 22, 2018

Could you be more explicit?

I'll try below...

BTW, diofant is able to solve problem from sympy/sympy#15520.

Exactly. So if a possible future improvement to Diofant's dsolve meant that the third DE gave a answer without RootOf (as is theoretically possible, even if roots is not the right way to do it) then there would be a gap in the test coverage. (Its not really a big deal, just trying to share in both directions when I learn something).

Thanks for the tip about roots not returning all answers.

This comment has been minimized.

Owner

skirpichev replied Nov 22, 2018

without RootOf (as is theoretically possible

I'm not a fan of returning answers without RootOf's by default. This tends to produce rather complex answers, except for trivial cases (quadratic equations and so on). And in fact - this could be extremely slow for anything else then simple cubic/quartics/quintics. As far as I know, other CAS turn off such ability by default and there is no practical algorithms to solve this problem for high enough degree (where "high" is around 10, I think, Kalevi Suominen may know better about the state of art).

In short, there are performance issues, that prohibit enabling such expansions by default (like integrals evaluation, see diofant#281). And also such answers will be too complex in most cases.

As I said, there may be a dedicated method like ToRadicals in the Mathematica. Which even may be used by simplify() internally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment