I had a quick look, a few comments:

If you have

some_function(x, y, z,
    long_long_long_long_long_long_long_long_var)

you don't need to escape the newline. Using parentheses, you don't need to
escape line ends in your code at all.

numbered_symbols() could be used by cse(), reducing redundancy.

I really like the extensive documentation and tests.

Two tests are failing (on 64 bit platform):

__
sympy/solvers/tests/test_ode.py:test_nth_linear_constant_coeff_homogeneous
__
  File "/home/one/src/sympy/sympy/solvers/tests/test_ode.py", line 564, in
test_nth_linear_constant_coeff_homogeneous
    assert dsolve(eq6, f(x)) == sol6
AssertionError
________________________________________________________________________________
 
sympy/solvers/tests/test_ode.py:test_nth_linear_constant_coeff_undetermined_coefficients

  File "/home/one/src/sympy/sympy/solvers/tests/test_ode.py", line 821, in
test_nth_linear_constant_coeff_undetermined_coefficients
    assert dsolve(eq16, f(x), hint=hint) == sol16
AssertionError

The output of py.test (possibly more useful, a bit confusing due to Eq()'s
repr):

>       assert dsolve(eq6, f(x)) == sol6
E       assert f(x) == C1*exp(-x - x*2**(1/2)) + C2*exp(-x + x*2**(1/2)) ==
f(x) == C2*exp(-x - x*2**(1/2)) + C1*exp(-x + x*2**(1/2))
E        +  where f(x) == C1*exp(-x - x*2**(1/2)) + C2*exp(-x + x*2**(1/2))
= dsolve(-f(x) + 2*D(f(x), x) + D(f(x), x, x) == 0, f(x))
E        +    where f(x) = f(x)

>       assert dsolve(eq16, f(x), hint=hint) == sol16
E       assert f(x) == (C1 - x**2/8)*cos(2*x) + (C2 + x/16)*sin(2*x) == f(x)
== (C2 - x**2/8)*cos(2*x) + (C1 + x/16)*sin(2*x)
E        +  where f(x) == (C1 - x**2/8)*cos(2*x) + (C2 + x/16)*sin(2*x) =
dsolve(4*f(x) - x*sin(2*x) + D(f(x), x, x), f(x),
hint='nth_linear_constant_coeff_undetermined_coefficients')
E        +    where f(x) = f(x)

So it seems it's rather about the constants.

Vinzent


2009/8/20 Aaron S. Meurer <asmeu...@gmail.com>

> You can repull, I have fixed the checkodesol thing, as well as some other
> bugs.
> Aaron Meurer
>
> On Aug 20, 2009, at 2:58 PM, Vinzent Steinberg wrote:
>
> 2009/8/20 Aaron S. Meurer <asmeu...@gmail.com>
>
>>
>> On Aug 20, 2009, at 11:19 AM, Vinzent Steinberg wrote:
>>
>>
>> 2009/8/20 Aaron S. Meurer <asmeu...@gmail.com>
>>
>>>
>>> On Aug 20, 2009, at 5:23 AM, Vinzent Steinberg wrote:
>>>
>>> Thank you, this is an impressive result. :)
>>>
>>>  2009/8/20 Aaron S. Meurer <asmeu...@gmail.com>
>>>
>>>>
>>>> I have finished up my Google Summer of Code 2009 project, which was to
>>>> implement various ODE solvers in SymPy.  Please pull fromhttp://
>>>> github.com/asmeurer/sympy/tree/odes-review
>>>> .  Note that I have elected to do only moderate rebasing from my
>>>> original tree (branch odes), so you should really only be reviewing
>>>> the HEAD.
>>>>
>>>> This includes the following features:
>>>>
>>>> - All ODE solving now happens in ode.py, instead of solvers.py where
>>>> it was crowding stuff.
>>>> - dsolve() remains the function to use to solve an ODE.  The syntax is
>>>> the same for default solving: dsolve(ode, func).
>>>> For example:
>>>>  >>> dsolve(diff(f(x), x, 2) - 1, f(x))
>>>> f(x) == C1 + C2*x + x**2/2
>>>>
>>>> dsolve() always returns an Equality instance, because it very often
>>>> cannot solve for f(x) and must return an implicit solution.
>>>>
>>>> - The function classify_ode() was added.  This function classifies
>>>> ODEs into the various methods (or "hints", as I also call them) that
>>>> dsolve() can use to solve them.
>>>> For example:
>>>>  >>> classify_ode(f(x).diff(x) - 1, f(x))
>>>> ('separable', '1st_exact', '1st_linear', 'Bernoulli',
>>>> '1st_homogeneous_coeff_best',
>>>> '1st_homogeneous_coeff_subs_indep_div_dep',
>>>> '1st_homogeneous_coeff_subs_dep_div_indep',
>>>> 'nth_linear_constant_coeff_undetermined_coefficients',
>>>> 'nth_linear_constant_coeff_variation_of_parameters',
>>>> 'separable_Integral', '1st_exact_Integral', '1st_linear_Integral',
>>>> 'Bernoulli_Integral',
>>>> '1st_homogeneous_coeff_subs_indep_div_dep_Integral',
>>>> '1st_homogeneous_coeff_subs_dep_div_indep_Integral',
>>>> 'nth_linear_constant_coeff_variation_of_parameters_Integral')
>>>>
>>>> - The function deriv_degree() remains the function to use for
>>>> determining the degree, or order, of an ODE.
>>>> For example:
>>>>  >>> deriv_degree(f(x).diff(x, 5) + sin(x) + f(x).diff(x, 2), f(x))
>>>> 5
>>>>
>>>> - The new function checkodesol() checks if a solution to an ODE is
>>>> valid by substituting it in and either returning True if it finds 0
>>>> equivalence or the expression that it could not reduce to 0.
>>>> For example:
>>>>  >>> checkodesol(f(x).diff(x, 2) - 1, f(x), Eq(f(x), C1 + C2*x +
>>>> x**2/2))
>>>> True
>>>>  >>> checkodesol(f(x).diff(x, 2) - 1, f(x), Eq(f(x), C1 + C2*x + x**2))
>>>> 1
>>>
>>>
>>> This is problematic, because this way
>>>
>>> if not checkodesol(...):
>>>     print 'not a solution'
>>>
>>> does not work, because most expressions evaluate to True. I'd prefer
>>> returning just 0 instead of True, this is more consistent and useful.
>>>
>>>
>>> Well, the idea is that sometimes an expression will be equivalent to 0,
>>> but simplify() cannot do it.  So we return the result to the user, who can
>>> then do his own checking to see if it is 0.  This is more or less what
>>> Maple's
>>>
>>> If you read the docstring, you can see that "if checkodesol()" is bad
>>> because it will always be True (unless it fails), but "if checkodesol() is
>>> True" will only evaluate to True if it returns the bool True.
>>>
>>
>> I think this is hacky and misleading, "is True" is not beautiful. I'd
>> really return just 0, the user can check for this in his code, and it would
>> be much more consistent. Returning True makes imho no sense when False can't
>> be returned.
>>
>>
>> False is returned when the function fails (none of the checking methods
>> can be applied for whatever reason).  I think we need to return the result
>> somehow, because it is actually quite often that the result is 0 but
>> simplify() cannot do it.  I am open to changing the output, but I definitely
>> think we need to maintain this behavior somehow.  Maybe there be some kind
>> of keyword argument that does something.
>>
>
> What's wrong about just returning the result? Be it 0, or something that
> does not simplify to 0 or is not 0.
>
> I'm also fine with the tuple.
>
>
>>
>> Another possibility would be to return a tuple, either (True, 0) or
>> (False, <whatever isn't 0>), though as far as ugly goes, I think that is
>> more ugly than my current result. :)
>>
>>
>>
>>>
>>>
>>>
>>>>
>>>>
>>>> - The function separatevars(), which I wrote to help with the 1st
>>>> order separable method, attempts to intelligently separate variables
>>>> an expression  multiplicatively.
>>>> For example:
>>>>  >>> separatevars(x**2 + x**2*sin(y))
>>>> x**2*(1 + sin(y))
>>>>  >>> separatevars(exp(x + y))
>>>> exp(x)*exp(y)
>>>>
>>>> This function resides in simplify.py.
>>>>
>>>> - The new function homogeneous_order(), which I wrote to help with the
>>>> 1st order ODE with homogeneous coefficients method, determines the
>>>> homogeneous order of an expression.  An expression F(x, y, ...) is
>>>> homogeneous of order n if F(x*t, y*t, t*...) == t**n*F(x, y, ...).
>>>> For example:
>>>>  >>> homogeneous_order(x**2*sqrt(x**2 + y**2) + x*y**2*sin(x/y), x, y)
>>>> 3
>>>>
>>>> - dsolve() will simplify arbitrary constants, and renumber them in
>>>> order in the expression.  You should never see something like x + 2*C1
>>>> or 1 + C2*x + C1*x**2 returned from dsolve().  You can disable this by
>>>> using simplify=False in dsolve().
>>>>
>>>> - Internally, the solving engine has been completely refactored.  The
>>>> old way was to match the ODE to various solvers in a predetermined
>>>> order, and as soon as it matched one, it would solve it using that
>>>> method and return a result.  This had several problems with it.
>>>>
>>>> First, several ODEs can be solved with more than one method (c.f. the
>>>> simple ODE in the classify_ode() example above), and sometimes you
>>>> want to see the result from a different method.
>>>>
>>>> Second, sometimes the default solver would return a result that was
>>>> much more complex than another one would have returned if the ODE had
>>>> gotten to it.  For example, it may return an implicit solution that is
>>>> difficult for solve() to solve, whereas the other method would return
>>>> the equation already solver.
>>>
>>>
>>> typo
>>>
>>>
>>> Sorry.  Don't worry, this is just a write up I made for this email.  It
>>> is not in the code anywhere. :)
>>>
>>
>> No problem, I just thought you might want to reuse it. It makes sense to
>> add all the things you wrote to a commit message I think.
>>
>>
>>>
>>> Thanks Vinzent.  I hope you can review this.
>>>
>>
>> I'll try to do it tonight, but I'm not an ODE expert, especially compared
>> to Ondrej. :)
>>
>>
>> That's OK.  You can tell at least if the code is clean, if there are
>> typos, and so on.  Do you know anything about ODEs at all?
>>
>
> I know what it is, and that a solution should be a function satisfying the
> equation. And I know what it's used for. So my knowledge is only
> superficial.
>
>
>>  The methods I have implemented are only ones you would learn in a first
>> semester ODE course anyway (that's all I have taken).
>>
>
> I just don't know the methods (I did never try to solve an ODE, except for
> direct integration of course).
>
> Vinzent
>
>
>
>
>
> >
>

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups 
"sympy-patches" group.
To post to this group, send email to sympy-patches@googlegroups.com
To unsubscribe from this group, send email to 
sympy-patches+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/sympy-patches?hl=en
-~----------~----~----~----~------~----~------~--~---

Reply via email to