Skip to content
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

Update ode.py #15930

Closed
wants to merge 8 commits into from
Closed

Update ode.py #15930

wants to merge 8 commits into from

Conversation

Teut2711
Copy link
Contributor

@Teut2711 Teut2711 commented Feb 5, 2019

References to other Issues or PRs

Brief description of what is fixed or changed

Other comments

Release Notes

  • solvers
    • ode
      • added a wrapper
  • functions
    • 2 functions added

@sympy-bot
Copy link

sympy-bot commented Feb 5, 2019

Hi, I am the SymPy bot (v136). I'm here to help you write a release notes entry. Please read the guide on how to write release notes.

Your release notes are in good order.

Here is what the release notes will look like:

This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.4.

Note: This comment will be updated with the latest check if you edit the pull request. You need to reload the page to see it.

Click here to see the pull request description that was parsed.

<!-- Your title above should be a short description of what
was changed. Do not include the issue number in the title. -->

#### References to other Issues or PRs
<!-- If this pull request fixes an issue, write "Fixes #15922 " in that exact
format, e.g. "Fixes #1234". See
https://github.com/blog/1506-closing-issues-via-pull-requests . Please also
write a comment on that issue linking back to this pull request once it is
open. -->


#### Brief description of what is fixed or changed


#### Other comments


#### Release Notes

<!-- Write the release notes for this release below. See
https://github.com/sympy/sympy/wiki/Writing-Release-Notes for more information
on how to write release notes. The bot will check your release notes
automatically to see if they are formatted correctly. -->

<!-- BEGIN RELEASE NOTES -->
* solvers
    * ode
        * added a wrapper
* functions
    *  2 functions added
<!-- END RELEASE NOTES -->

@Teut2711
Copy link
Contributor Author

Teut2711 commented Feb 5, 2019

Hi,
Hope you can see the changes now.

@oscarbenjamin
Copy link
Collaborator

Hi @XtremeGood we don't want to use a decorator for this. This is one of many possible methods used in solving ODEs and decorators don't scale very well if we need many of them. Please carefully read what I said here: #15922 (comment)

@Teut2711
Copy link
Contributor Author

Teut2711 commented Feb 5, 2019 via email

@oscarbenjamin
Copy link
Collaborator

You are trying to get your code integrated into the ode module which is almost 10000 lines of code already. Your new code needs to fit in with the rest of that so it needs to be written a particular way. If you take time just to read through the code in the ode module you might learn more about how it fits together.

If you try it and get stuck then you can push the code here and I'll take a look. (I have pretty clear idea about how this can be done.)

Can you please comment using the web interface rather than by email? It's okay if you can't reply immediately.

@Teut2711
Copy link
Contributor Author

Teut2711 commented Feb 5, 2019

Hi,
Yep,that's why I will try again today .For the time being I 'm preparing for a test.We've got surprised tests.It didn't happen yesterday, it may or may not happen today.

@Teut2711
Copy link
Contributor Author

Teut2711 commented Feb 8, 2019

Hi,
Sorry I had to study for the test but now I 'm free for the weekend and can get back to work now.

@Abdullahjavednesar Abdullahjavednesar added the PR: author's turn The PR has been reviewed and the author needs to submit more changes. label Feb 8, 2019
Integration by substituting min order derivative
@Teut2711
Copy link
Contributor Author

Teut2711 commented Feb 8, 2019 via email

@oscarbenjamin
Copy link
Collaborator

This looks better.

Now that you have updated the PR the tests are running. The Travis tests are not completed yet but if you look now you can already see that some are failing. Click the "Details" button below:.

I looked quickly and the errors I saw are because the function order_reducing_substitution should have an underscore at the start of its name. You're calling it as _ order_reducing_substitution.

@Teut2711
Copy link
Contributor Author

Teut2711 commented Feb 8, 2019

Thanks for informing. I've changed that.

sympy/solvers/ode.py Outdated Show resolved Hide resolved
@oscarbenjamin
Copy link
Collaborator

There are still more errors. You can see them if you click the "Details" button below. I think you would notice the errors before committing if you tested the code yourself though.

Try the example from #15881. If that isn't working then the code isn't ready.

@Teut2711
Copy link
Contributor Author

Teut2711 commented Feb 8, 2019

@oscarbenjamin
Yep, I have heard about performing the unit tests before submitting any code but this seemed to be a simple code and I've just switched from default python idle to visual studio code.I will soon learn all that stuff.

@oscarbenjamin
Copy link
Collaborator

I'm not just talking about unit tests. The errors I can see in the Travis logs are from mistakes that you would have seen if you had tested this with a single example.

If I test your code to try the ODE from #15881 I get this:

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

In [2]: dsolve(eq)                                                                                                                
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-2-42dc4d2e7b25> in <module>
----> 1 dsolve(eq)

~/current/sympy/sympy/sympy/solvers/ode.py in dsolve(eq, func, hint, simplify, ics, xi, eta, x0, n, **kwargs)
    645         hints = _desolve(eq, func=func,
    646             hint=hint, simplify=True, xi=xi, eta=eta, type='ode', ics=ics,
--> 647             x0=x0, n=n, **kwargs)
    648 
    649         eq = hints.pop('eq', eq)

~/current/sympy/sympy/sympy/solvers/deutils.py in _desolve(eq, func, hint, ics, simplify, **kwargs)
    203     if kwargs.get('classify', True):
    204         hints = classifier(eq, func, dict=True, ics=ics, xi=xi, eta=eta,
--> 205         n=terms, x0=x0, prep=prep)
    206 
    207     else:

~/current/sympy/sympy/sympy/solvers/ode.py in classify_ode(eq, func, dict, ics, **kwargs)
   1360         # integrals e.g.:
   1361         # d^3/dx^3(x y) = F(x)
-> 1362         r = _check_substitution_type(reduced_eq, func)
   1363         if r["solutions"]:
   1364             matching_hints['order_reducing_substitution'] = r

~/current/sympy/sympy/sympy/solvers/ode.py in _check_substitution_type(eq, func)
   4010     order_to_subs = 0
   4011     D = Dummy()
-> 4012     for nsubs_try in range(1,ode_order(eq)):
   4013         if reduced_eq.subs(f(x).diff(x, nsubs_try), D).has(f(x)):
   4014             break

TypeError: ode_order() missing 1 required positional argument: 'func'

@Teut2711
Copy link
Contributor Author

Teut2711 commented Feb 8, 2019

It will take time.It needs a lot of modifications.

@oscarbenjamin
Copy link
Collaborator

Okay well take your time. Did you try the example I just showed before sending the last two commits?

Test the code locally (on your computer). Don't push it here until you think it might be working.

@Teut2711
Copy link
Contributor Author

Teut2711 commented Feb 8, 2019

Code breaks at _desolve in utils but that it connected to classify_ode.I got 1 bug though.

@oscarbenjamin
Copy link
Collaborator

If you get stuck you can push what you have so far and ask for help.

@Teut2711
Copy link
Contributor Author

Teut2711 commented Feb 10, 2019 via email

@Teut2711
Copy link
Contributor Author

Teut2711 commented Feb 10, 2019

Hi,
I ran the code in debug mode in visual studio debug mode.It's skipping my function and not including it in hints.I ran debug on dsolve, classify_ode, and _desolve. Since the debug mode works only on one file I couldn't see how dsolve calls _desolve which further calls classify_ode. I' m stuck.

@oscarbenjamin
Copy link
Collaborator

If you push what you have right now I can probably tell you why it's not working.

Can you also show what you tried and what the error message was (the whole error message with traceback)? The traceback should show you which line of which function is calling which other function.

Also you can search for function names within each .py file to see where they get called.

@Teut2711
Copy link
Contributor Author

Error in :
hints = _desolve(eq, func=func,
hint=hint, simplify=True, xi=xi, eta=eta, type='ode', ics=ics,
x0=x0, n=n, **kwargs)
Error:Exception has occurred: NotImplementedError
solve: Cannot solve xDerivative(f(x), x)**2 + Derivative(f(x), x, x)
File "C:\Users\Dell\Desktop\GitHub\sympy\sympy\solvers\ode.py", line 647, in dsolve
x0=x0, n=n, **kwargs)
File "C:\Users\Dell\Desktop\GitHub\sympy\sympy\solvers\ode.py", line 8709, in
print(dsolve(f.diff(x,2)+x
f.diff(x)**2))

@Teut2711
Copy link
Contributor Author

Teut2711 commented Feb 10, 2019

I tried running the function _desolve. There it directly jumps to "if not hints['default']".

@Teut2711
Copy link
Contributor Author

This ->>>Also you can search for function names within each .py file to see where they get called.

Search where?

@oscarbenjamin
Copy link
Collaborator

When pasting something like an error message or some code you should use backticks for markup: https://guides.github.com/features/mastering-markdown/

When I said you can search the function name I just meant in your editor. When looking at a file you can search within that file (presumably - I don't know what editor you're using).

return {'var':order_to_subs, 'solutions':True}

# Use repeated substitution until we do not have a function independent of derivative
def order_reducing_substitution(eq, func, order, match):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function should be called ode_order_reducing_substitution

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?I mean all other functions are named normally and everywhere I have used this name only.

Copy link
Contributor Author

@Teut2711 Teut2711 Feb 10, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And instead of markdown syntax would dpaste.com works?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the hint is called X then the corresponding function should be called ode_X.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the hint is called X then the corresponding function should be called ode_X.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No idea about dpaste.com. You should learn the markdown syntax to use github though

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use repeated substitution until we do not have a function independent of derivative

  • def ode_order_reducing_substitution(eq, func, order, match):

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use repeated substitution until we do not have a function independent of derivative

def ode_order_reducing_substitution(eq, func, order, match):

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, it will take multiple efforts to learn that styling. I will read that doc tomorrow.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing is that the r""" ....""" is coloring whole of the whole below it orange in editor which happened for quoted text in my editor.I removed the r and the code turned from orange to multi-colored.Strange isn't it?It seems as if the quotes aren't being actually closed even after closing them with """.On the other hand while debugging I could see the function names in the debugging window.I don't know if they were from the all_hints list or somewhere else but this is somewhat strange.

@Teut2711
Copy link
Contributor Author

I'm using visual studio code.I have heard about logging, and storing error logs but have never used them. I will have to learn it.

@oscarbenjamin
Copy link
Collaborator

You don't need to post things to see what they will look like. As I write this just above there is a "preview" tab. If I click that then it shows what the markdown I am editing will look like after posting.

The issue with colouring in your editor is probably specific to whichever editor you are using.

@@ -0,0 +1,70 @@
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file should not be here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi
I 'll delete them.


x = Symbol("x")
f = Function("f")(x)
print(dsolve(f.diff(x,2)+x*f.diff(x)**2))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code should not be here.

@asmeurer
Copy link
Member

The release notes need to be fixed.

@Teut2711 Teut2711 closed this Feb 11, 2019
@Teut2711 Teut2711 deleted the Order_reducing_substitution branch February 11, 2019 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: author's turn The PR has been reviewed and the author needs to submit more changes. solvers.dsolve
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants