give solution constants of ODEs unique names #16007

rwst · 2014-03-25T10:09:58Z

(this was reported as part of #6882)

... solving of ode y'=c*x is not correct, the free variable is messed up with a parameter, see sage-devel - thanks for Yuri Karadzhov

sage: from sage.calculus.calculus import symbolic_expression_from_maxima_string
sage: symbolic_expression_from_maxima_string('%c')
c
sage: c=var('c'); y=function('y',x); eq=diff(y,x)==c*x; eq
D[0](y)(x) == c*x
sage: desolve(eq,y,ivar=x)
1/2*c*x^2 + c

the answer should be something like 1/2cx² + c1

The fix depends on closing #8734. The c variable that comes from Sage can then be easily recognized, and the constant c should then be renamed to c1 or _C1.

Depends on #8734

CC: @kcrisman

Component: calculus

Author: Ralf Stephan

Branch: a8df107

Reviewer: Nils Bruin, Karl-Dieter Crisman

Issue created by migration from https://trac.sagemath.org/ticket/16007

The text was updated successfully, but these errors were encountered:

nbruin · 2014-03-26T06:23:25Z

comment:2

Actually, these variables are very easy to recognize any way:

sage: function('f',x)
f(x)
sage: var('c')
c
sage: W=diff(f(x),x,x)-f(x)+c
sage: V=diff(f(x),x)-f(x)+c
sage: maxima_calculus(V).ode2(f(x),x)
'f(x)=(c*%e^-x+%c)*%e^x
sage: maxima_calculus(W).ode2(f(x),x)
'f(x)=%k1*%e^x+%k2*%e^-x+c

As you can see, maxima creates these constants as %c, %k1, %k2, so there's no collision there. The collision only happens upon conversion back to sage. So we make the conversion more intelligent there wouldn't be an issue at all. Using sage.interfaces.maxima_lib.max_to_sr this would be fairly straightforward.

rwst · 2014-03-26T15:45:45Z

Branch: u/rws/ticket/16007

rwst · 2014-03-26T15:47:09Z

comment:4

Indeed, many thanks. With this easy patch the output is now

sage: sage: x=function('x',t)
sage: sage: var('c')
c
sage: sage: desolve(diff(x,t)+2*x==t^2-2*t+c,x,ivar=t).expand()
1/2*t^2 + _C*e^(-2*t) + 1/2*c - 3/2*t + 3/4

sage: desolve(diff(f(x),x,x)-f(x),f(x))
_K2*e^(-x) + _K1*e^x

Any more variables?

New commits:

`ee630f5`	`add %c,%k1,%k2 to recognized maxima objects`

rwst · 2014-03-26T15:47:09Z

Commit: ee630f5

rwst · 2014-03-26T15:47:09Z

Changed dependencies from #8734 to none

rwst · 2014-03-26T15:47:09Z

Author: Ralf Stephan

zimmermann6 · 2014-03-27T09:05:56Z

comment:5

seems to be a duplicate of #9421.

Paul

rwst · 2014-03-27T10:17:46Z

comment:6

Yes. So, are you okay with this solution applied to #9421, instead of the warning? If so, then I will make a reviewer's patch there and invalid this ticket.

zimmermann6 · 2014-03-27T11:04:59Z

comment:7

yes I agree with this solution, but I won't have time quite soon to check the doctests still pass.

Paul

kcrisman · 2014-03-27T16:38:56Z

comment:8

It might be worth doing a few things. First, you must have documentation testing this fix - especially since people will be confused by underscore variables without some documentation! Also, how many different things can show up? Just c, k1, k2, or also c1 et al, or k3, ... ? It would be worth asking on the Maxima email list about what outputs can possibly happen (assuming it's working - I was at some point subscribed to the "new" version but now am apparently not again).

But I agree it's very unlikely for anyone to have an underscore variable, so this could be a good compromise... there remains the issue of substituting in for non-Sageified variables.

rwst · 2014-06-05T15:39:53Z

comment:11

Deleted misplaced comment.

rwst · 2014-06-06T08:45:00Z

comment:12

Maxima-discuss list mail:

On 05-06-2014 16:36, Ralf Stephan wrote:
>Sage uses Maxima to solve ODEs. To do that bug free we would
>need to know which constant vars apart from %c, %k1, %k2 are
>returned by Maxima. Can you please help?
looking at the source code of the programs in share/diffequations,
it seems to me that those 3 are the only ones. I don't know if you
have other programs in mind, apart from those in share/diffequations.

Regards,
Jaime

I take it then the ticket is complete for review.

kcrisman · 2014-06-06T14:35:08Z

comment:13

Thanks for doing that due diligence. Yes, I believe these only arise from a contrib package in Maxima.

I take it then the ticket is complete for review.

Well... see in comment:8

First, you must have documentation testing this fix - especially since people will be confused by underscore variables without some documentation!

It's really unfortunate with the underscores. But I hate to think of checking whether C, C1, C2 etc. were defined and then using whatever the next one was. Anyway, unless Nils (who knows this stuff better than me) objects to your solution (quoting #9421 comment:9)

(with the righter solution being: making sage's "forget the %" more intelligent, so that collisions can be avoided)

then as I said, this is definitely much better than leaving it 'as is'.

sagetrac-git · 2014-06-07T06:53:26Z

Changed commit from ee630f5 to 630ea92

sagetrac-git · 2014-06-07T06:53:26Z

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

`1e9b54b`	`8734: more doctests fixed`
`06b9528`	`8734: fix doctests`
`6fffc21`	`Merge branch 'develop' into t/8734/ticket/8734-1`
`3d74cad`	`8734: rename private function, doctest it`
`3180e7b`	`Merge branch 'develop' into t/8734/ticket/8734-1`
`cec9681`	`8734: merge conflicts`
`f702088`	`take back unrelated change`
`2d31fb1`	`8734: doc cosmetics`
`100c8eb`	`Merge branch 't/8734/ticket/8734-1' into t/16007/ticket/16007`
`630ea92`	`16007: fix doctests; add documentation`

rwst · 2014-06-07T06:54:33Z

comment:15

Merged in #8734. Thanks for the doctest reminder.

kcrisman · 2014-06-08T00:46:51Z

comment:16

Merged in #8734. Thanks for the doctest reminder.

No problem. Do you think it's useful to add an example where there might have otherwise been a collision of names, or is that superfluous?

Also, why is the doctest

sefms('%k1*%x + %k2*%y + %c')

and not

sefms('%k1*x + %k2*y + %c')

I'm not sure what the % is doing on the variables, though I don't doubt Sage would strip those percents off, if I recall that part of the code correctly (it's been a while, though).

sagetrac-git · 2014-06-08T07:07:31Z

Branch pushed to git repo; I updated commit sha1. New commits:

`92a3fa3`	`16007: make doctest test intended case`

sagetrac-git · 2014-06-08T07:07:31Z

Changed commit from 630ea92 to 92a3fa3

rwst · 2014-06-19T03:12:14Z

comment:35

Well, now there is a code point where the behaviour can be changed. So, again, is there anything missing?

kcrisman · 2014-06-24T15:48:52Z

comment:36

though I don't know how one would typeset the underscore in any case.

Turns out it currently just keeps the underscore.

This change could minutely slow down latex for variables that start with underscores, or if the symtable gets huge... but that is probably silly to worry about for now, neither are likely in the near future. Also may help with #6882. So I'm happy with this commit.

So, again, is there anything missing?

Doctest (presumably just in latex_variable_name) for the latexing. I realize this seems like a lot of rounds, but I've found the review process very helpful in the past, anyway. (And I'm sorry I haven't time to check out the last commit of #8734 yet.)

sagetrac-git · 2014-06-25T05:41:09Z

Changed commit from fe2a0f0 to 28a08aa

sagetrac-git · 2014-06-25T05:41:09Z

Branch pushed to git repo; I updated commit sha1. New commits:

`7918e73`	`Merge branch 'develop' into t/16007/ticket/16007`
`28a08aa`	`16007: doctest latexification; move author info to top`

rwst · 2014-06-25T05:43:42Z

comment:38

Replying to @kcrisman:

So, again, is there anything missing?

Doctest (presumably just in latex_variable_name) for the latexing. I realize this seems like a lot of rounds, but I've found the review process very helpful in the past, anyway. (And I'm sorry I haven't time to check out the last commit of #8734 yet.)

I appreciate your thoroughness, and thanks for the review!

kcrisman · 2014-06-25T13:37:43Z

comment:39

Okay, if doctests pass (currently attempting to build the branch but it will take a while) then I'm happy with this. Thanks for working on so many symbolics/calculus things recently.

vbraun · 2014-06-26T02:46:57Z

comment:41

Doctests don't pass (try make ptestlong)

sage -t --long src/doc/en/tutorial/tour_algebra.rst
**********************************************************************
File "src/doc/en/tutorial/tour_algebra.rst", line 156, in doc.en.tutorial.tour_algebra
Failed example:
    desolve(DE, [x,t])
Expected:
    (c + e^t)*e^(-t)
Got:
    (_C + e^t)*e^(-t)
**********************************************************************

kcrisman · 2014-06-26T03:13:42Z

comment:42

Nuts!!! I could have sworn I checked everything... and now I see why long tests on #8734 are taking so long too, I did make testlong and not make ptestlong. Very sorry about this.

sagetrac-git · 2014-06-26T08:56:04Z

Changed commit from 28a08aa to 4add98a

sagetrac-git · 2014-06-26T08:56:04Z

Branch pushed to git repo; I updated commit sha1. New commits:

`2c68026`	`8734: adapt doctest to recent change of output`
`793bb05`	`8734: reraise in "other" cases with bare "raise"`
`b5857b7`	`Merge branch 'u/rws/ticket/8734-1' of trac.sagemath.org:sage into t/16007/ticket/16007`
`4add98a`	`16007: fix documentation examples`

rwst · 2014-06-26T08:59:01Z

comment:44

Not all of #8734 was merged. Sorry for the inconvenience.

vbraun · 2014-06-26T12:24:54Z

comment:45

So has the code from #8734 been reviewed? If not make it a dependency here so I don't accidentally merge it.

rwst · 2014-06-26T12:30:07Z

Dependencies: #8734

sagetrac-git · 2014-07-02T15:22:19Z

Branch pushed to git repo; I updated commit sha1. New commits:

`6bfa305`	`Merge branch 'develop' into t/16007/ticket/16007`
`a195985`	`8734: remove superfluous code`
`67426e1`	`Merge branch 'u/rws/ticket/8734-1' of trac.sagemath.org:sage into t/16007/ticket/16007`

sagetrac-git · 2014-07-02T15:22:19Z

Changed commit from 4add98a to 67426e1

kcrisman · 2014-07-02T15:48:08Z

comment:48

I am not going to check whether this passes tests, but at any rate the changes most recent seem fine, although I don't know why the non-doctest pieces had to have the constant C instead of c (but I don't really care about that). So if anyone checks it passes tests we are good here, since #8734 is positive review.

sagetrac-git · 2014-07-03T06:22:51Z

Changed commit from 67426e1 to a8df107

sagetrac-git · 2014-07-03T06:22:51Z

Branch pushed to git repo; I updated commit sha1. New commits:

`65107d4`	`Merge branch 'develop' into t/8734/ticket/8734-1`
`4e07383`	`6882: add rules for e, i, I`
`e5a5343`	`6882: do word search/replace for symtable keys`
`4c1b0eb`	`6882: correction to previous commit`
`518de3e`	`6882: add symable rules for e,i,I; fix maxima_var; add doctests`
`7a6696b`	`Merge branch 'u/rws/bugs_in_conversion_of_variable_names_from_maxima_to_sage' of trac.sagemath.org:sage into t/8734/ticket/8734-1`
`a8df107`	`Merge branch 't/8734/ticket/8734-1' into t/16007/ticket/16007`

rwst · 2014-07-03T06:26:16Z

comment:50

Thanks. Pulled in changes from dependencies. I'll run a patchbot on this now.

vbraun · 2014-07-08T13:06:29Z

Changed branch from u/rws/ticket/16007 to a8df107

fchapoton · 2015-09-06T13:08:54Z

Changed commit from a8df107 to none

rwst added this to the sage-6.2 milestone Mar 25, 2014

rwst added c: calculus labels Mar 25, 2014

rwst added the s: needs review label Mar 26, 2014

rwst added s: needs work and removed s: needs review labels Apr 4, 2014

sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014

rwst added s: needs review and removed s: needs work labels Jun 6, 2014

kcrisman added s: positive review and removed s: needs review labels Jun 25, 2014

vbraun added s: needs work and removed s: positive review labels Jun 26, 2014

rwst added s: needs review and removed s: needs work labels Jun 26, 2014

vbraun removed the s: needs review label Jul 8, 2014

vbraun closed this as completed in 7267621 Jul 8, 2014

This comment has been minimized.

Sign in to view

give solution constants of ODEs unique names #16007

give solution constants of ODEs unique names #16007

Comments

rwst commented Mar 25, 2014

nbruin commented Mar 26, 2014

rwst commented Mar 26, 2014

rwst commented Mar 26, 2014

rwst commented Mar 26, 2014

rwst commented Mar 26, 2014

rwst commented Mar 26, 2014

zimmermann6 commented Mar 27, 2014

rwst commented Mar 27, 2014

zimmermann6 commented Mar 27, 2014

kcrisman commented Mar 27, 2014

rwst commented Jun 5, 2014

rwst commented Jun 6, 2014

kcrisman commented Jun 6, 2014

sagetrac-git mannequin commented Jun 7, 2014

sagetrac-git mannequin commented Jun 7, 2014

rwst commented Jun 7, 2014

kcrisman commented Jun 8, 2014

sagetrac-git mannequin commented Jun 8, 2014

sagetrac-git mannequin commented Jun 8, 2014

rwst commented Jun 19, 2014

kcrisman commented Jun 24, 2014

sagetrac-git mannequin commented Jun 25, 2014

sagetrac-git mannequin commented Jun 25, 2014

rwst commented Jun 25, 2014

kcrisman commented Jun 25, 2014

vbraun commented Jun 26, 2014

kcrisman commented Jun 26, 2014

sagetrac-git mannequin commented Jun 26, 2014

sagetrac-git mannequin commented Jun 26, 2014

rwst commented Jun 26, 2014

vbraun commented Jun 26, 2014

rwst commented Jun 26, 2014

sagetrac-git mannequin commented Jul 2, 2014

sagetrac-git mannequin commented Jul 2, 2014

kcrisman commented Jul 2, 2014

sagetrac-git mannequin commented Jul 3, 2014

sagetrac-git mannequin commented Jul 3, 2014

rwst commented Jul 3, 2014

vbraun commented Jul 8, 2014

fchapoton commented Sep 6, 2015

This comment has been minimized.