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

dsolve: expm/jordan solver #286

Merged
merged 10 commits into from Jun 20, 2016

Conversation

4 participants
@skirpichev
Copy link
Collaborator

skirpichev commented Jun 4, 2016

I hope @cbm755 ok if I continue his work here.

  • cleanup classify_sysode, drop redundant/untested code
  • cleanup tests/doctests
  • support mass matrix case, but see #293
  • don't simplify answer at all? (i.e. with sin/sinh)
  • drop unused sysode handlers, if any
  • good references (Hairer I?)
  • rename check_linear_order1_jordan-> check_linear_neq_order1,
    sysode_linear_order1_jordan -> sysode_linear_neq_order1
  • rebase
@cbm755

This comment has been minimized.

Copy link
Contributor

cbm755 commented Jun 6, 2016

No I don't mind, I wish I'd kept at this and fixed it in SymPy: I will still try.

Duplicated effort makes me sad :(

@skirpichev

This comment has been minimized.

Copy link
Collaborator

skirpichev commented Jun 7, 2016

No I don't mind

Good news, thank you for reply.

I will still try.

Your code was sitting around in the pr queue for years without review... So, good luck.

Duplicated effort makes me sad :(

Are you about this project? Yeah, but you know my history, at least partially. That wasn't my idea to fork the SymPy, but now it is what it is. For good or bad - now there is a different project with different goals and priorities.

@cbm755

This comment has been minimized.

Copy link
Contributor

cbm755 commented Jun 7, 2016

Oh sorry, I only meant this patch! Did not mean your project.

@skirpichev skirpichev changed the title dsolve: expm/jordan solver + fixes [wip] dsolve: expm/jordan solver + fixes Jun 8, 2016

cbm755 and others added some commits Apr 8, 2015

dsolve: Implement a general linear system solver
Uses Jordan form.

Edited by skirpichev.

Signed-off-by: Sergey B Kirpichev <skirpichev@gmail.com>
WIP: special treatment of complex conjugate pairs to give sin/cos
Edited by skirpichev.

Signed-off-by: Sergey B Kirpichev <skirpichev@gmail.com>
dsolve: hack to use new jordan solver
This implement a check method for new jordan block solver.

Edited by skirpichev.

Signed-off-by: Sergey B Kirpichev <skirpichev@gmail.com>
dsolve: some XFAIL tests now pass, minor edits for another test
Edited by skirpichev.

Signed-off-by: Sergey B Kirpichev <skirpichev@gmail.com>
Fix tests (for new linear solver)
Some checks were replaced with checksysodesol, instead of
manual subs/simplify.

@skirpichev skirpichev force-pushed the skirpichev:dsolve-linear branch from d496ced to 82c89ec Jun 18, 2016

@skirpichev skirpichev changed the title [wip] dsolve: expm/jordan solver + fixes dsolve: expm/jordan solver Jun 18, 2016

@skirpichev

This comment has been minimized.

Copy link
Collaborator

skirpichev commented Jun 18, 2016

Ok, now I think it's ready to be merged (to replace old horrible LODE solver). Rewriting to sin/cos was reverted back, perhaps I'll add this later.

Solutions sometimes seems (with and without sin/cos rewriting) to be more complex (see tests changes), but I believe it worth, as new solver is much more simple.

Please tell me if you would like to review changes.

@skirpichev skirpichev merged commit c396ae2 into diofant:master Jun 20, 2016

3 checks passed

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

@skirpichev skirpichev deleted the skirpichev:dsolve-linear branch Jun 20, 2016

@skirpichev

This comment has been minimized.

Copy link
Collaborator

skirpichev commented Jun 20, 2016

Anyway, thank you.

@skirpichev skirpichev modified the milestone: 0.8.0 Sep 9, 2016

@vishalg2235

This comment has been minimized.

Copy link

vishalg2235 commented Mar 8, 2018

@skirpichev @cbm755 I have made same changes as done here but my results are coming different.

like here

>>> eq3 = (Eq(diff(x(t),t), x(t) + y(t)), Eq(diff(y(t),t), -2*x(t) + 2*y(t)))
>>> dsolve(eq3)
[Eq(x(t), C1*(-2*sqrt(7)*I*(1/4 + sqrt(7)*I/4)*exp(t*(3 - sqrt(7)*I)/2)/7 + 2*sqrt(7)*I*(1/4 - sqrt(7)*I/4)*exp(t*(3 + sqrt(7)*I)/2)/7) + C2*((1/4 + sqrt(7)*I/4)*(1/2 + sqrt(7)*I/14)*exp(t*(3 - sqrt(7)*I)/2) + (1/4 - sqrt(7)*I/4)*(1/2 - sqrt(7)*I/14)*exp(t*(3 + sqrt(7)*I)/2))), Eq(y(t), C1*(-2*sqrt(7)*I*exp(t*(3 - sqrt(7)*I)/2)/7 + 2*sqrt(7)*I*exp(t*(3 + sqrt(7)*I)/2)/7) + C2*((1/2 + sqrt(7)*I/14)*exp(t*(3 - sqrt(7)*I)/2) + (1/2 - sqrt(7)*I/14)*exp(t*(3 + sqrt(7)*I)/2)))]

and here

>>> dsolve(e)
[Eq(f(x), C1*exp(-9*I*x)), Eq(g(x), C2*exp(-4*I*x))]

I think the form of answer is coming different.

@skirpichev

This comment has been minimized.

Copy link
Collaborator

skirpichev commented Mar 8, 2018

Can't you just check math?

@cbm755

This comment has been minimized.

Copy link
Contributor

cbm755 commented Mar 8, 2018

I don't know what you're trying to say... Are you reviewing these commits and you think you've found a problem? I don't know what "form of answer is coming different" means. Is the answer right or wrong? As @skirpichev says, you can check it...

@vishalg2235

This comment has been minimized.

Copy link

vishalg2235 commented Mar 9, 2018

Yes! Math is correct I'am just wondered why C1*(-2*sqrt(7)*E**(t*(3 - sqrt(7)*I)/2)*I*(1/4 + sqrt(7)*I/4)/7 this has been changed to C1*(-2*sqrt(7)*I*(1/4 + sqrt(7)*I/4)*exp(t*(3 - sqrt(7)*I)/2)/7 . E is replaced by exp, terms with I is coming before exp in my results. Thats what I meant by 'form of answer'.
And in second case -4*C1 is replaced by C1, Although it's correct as it is an arbitrary constant.

@asmeurer

This comment has been minimized.

Copy link
Contributor

asmeurer commented Mar 9, 2018

dsolve can be very sensitive to the form of solutions. That's why the dsolve tests were all written with checkodesol, to verify that the solution being tested is correct (it seems this practice was not extended to the systems tests). Generally, if the output form changes a little bit, you should just update the test (after verifying that the expression is still mathematically correct of course), unless it is significantly worse for some reason.

In the case of C1*(-2*sqrt(7)*E**(t*(3 - sqrt(7)*I)/2)*I*(1/4 + sqrt(7)*I/4)/7 vs. C1*(-2*sqrt(7)*I*(1/4 + sqrt(7)*I/4)*exp(t*(3 - sqrt(7)*I)/2)/7, these two expressions are identical. If you print them both in SymPy you will see that they both evaluate to the same thing. E** is the exact same thing as exp, and the order or arguments in a multiplication doesn't matter (Mul sorts them).

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