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

Autolev Parser Documentation 2 #15066

Merged
merged 24 commits into from Aug 21, 2018

Conversation

Projects
None yet
5 participants
@NikhilPappu
Contributor

NikhilPappu commented Aug 9, 2018

  • other
    • Added the Gotchas section in autolev_parser.rst in the mechanics part of the documentation.

NikhilPappu added some commits Aug 6, 2018

Added documentation for Autolev Parser under Mechanics
This PR contains the Introduction and Usage sections
@sympy-bot

This comment has been minimized.

Show comment
Hide comment
@sympy-bot

sympy-bot Aug 9, 2018

Hi, I am the SymPy bot (v128). 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:

  • other
    • Added the Gotchas section in autolev_parser.rst in the mechanics part of the documentation. (#15066 by @NikhilPappu)

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

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.

<!-- BEGIN RELEASE NOTES -->
- other
  - Added the Gotchas section in autolev_parser.rst in the mechanics part of the documentation. 
<!-- END RELEASE NOTES -->

Update

The release notes on the wiki have been updated.

sympy-bot commented Aug 9, 2018

Hi, I am the SymPy bot (v128). 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:

  • other
    • Added the Gotchas section in autolev_parser.rst in the mechanics part of the documentation. (#15066 by @NikhilPappu)

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

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.

<!-- BEGIN RELEASE NOTES -->
- other
  - Added the Gotchas section in autolev_parser.rst in the mechanics part of the documentation. 
<!-- END RELEASE NOTES -->

Update

The release notes on the wiki have been updated.

@NikhilPappu

This comment has been minimized.

Show comment
Hide comment
@NikhilPappu
Contributor

NikhilPappu commented Aug 9, 2018

@moorepants

This comment has been minimized.

Show comment
Hide comment
@moorepants

moorepants Aug 9, 2018

Member

Merge in master, since we've merged the first PR.

Member

moorepants commented Aug 9, 2018

Merge in master, since we've merged the first PR.

@NikhilPappu

This comment has been minimized.

Show comment
Hide comment
@NikhilPappu

NikhilPappu Aug 10, 2018

Contributor

@moorepants This PR is from autolev_parser_docs_2 to master. The first PR didn't get merged so can you please do that?

Contributor

NikhilPappu commented Aug 10, 2018

@moorepants This PR is from autolev_parser_docs_2 to master. The first PR didn't get merged so can you please do that?

@moorepants

This comment has been minimized.

Show comment
Hide comment
@moorepants

moorepants Aug 10, 2018

Member

Oh, I thought I merged it!

Member

moorepants commented Aug 10, 2018

Oh, I thought I merged it!

@moorepants

This comment has been minimized.

Show comment
Hide comment
@moorepants

moorepants Aug 10, 2018

Member

Ok, it is merged.

Member

moorepants commented Aug 10, 2018

Ok, it is merged.

@NikhilPappu

This comment has been minimized.

Show comment
Hide comment
@NikhilPappu

NikhilPappu Aug 10, 2018

Contributor

@moorepants Can you now review this PR and the 3rd one #15067 ? The Travis errors are because I reused a label. I'll fix that.
Also please take a look at #15013 and #15006 .

Contributor

NikhilPappu commented Aug 10, 2018

@moorepants Can you now review this PR and the 3rd one #15067 ? The Travis errors are because I reused a label. I'll fix that.
Also please take a look at #15013 and #15006 .

@moorepants

This comment has been minimized.

Show comment
Hide comment
@moorepants

moorepants Aug 10, 2018

Member

Yes, I'll get to them as I have time. I'm aware of all your PRs.

Member

moorepants commented Aug 10, 2018

Yes, I'll get to them as I have time. I'm aware of all your PRs.

NikhilPappu added some commits Aug 13, 2018

@NikhilPappu

This comment has been minimized.

Show comment
Hide comment
@NikhilPappu

NikhilPappu Aug 13, 2018

Contributor

@certik
Travis seems to have some issues. The build passed before but isn't passing now after just fixing typos.

Contributor

NikhilPappu commented Aug 13, 2018

@certik
Travis seems to have some issues. The build passed before but isn't passing now after just fixing typos.

@certik

This comment has been minimized.

Show comment
Hide comment
@certik

certik Aug 13, 2018

Member

I restarted the failed builds.

Member

certik commented Aug 13, 2018

I restarted the failed builds.

@NikhilPappu

This comment has been minimized.

Show comment
Hide comment
@NikhilPappu

NikhilPappu Aug 13, 2018

Contributor

@certik I have tried a few times. It doesn't seem to work.

Contributor

NikhilPappu commented Aug 13, 2018

@certik I have tried a few times. It doesn't seem to work.

@certik

This comment has been minimized.

Show comment
Hide comment
@certik

certik Aug 13, 2018

Member

@NikhilPappu can you please create a new issue with this bug? It looks like the "pip" no longer supports the legacy option.

Member

certik commented Aug 13, 2018

@NikhilPappu can you please create a new issue with this bug? It looks like the "pip" no longer supports the legacy option.

@moorepants

This a very helpful document and clearly explained. Nice work. I've left some feedback that I'd like addressed before merging.

Show outdated Hide outdated doc/src/modules/physics/mechanics/autolev_parser.rst
=======
- Don't use variable names that conflict with Python's reserved words.
This is one example where this is violated:

This comment has been minimized.

@moorepants

moorepants Aug 18, 2018

Member

You could throw warning if these variable names are found in the autolev code. I recommend opening an issue for this feature request.

@moorepants

moorepants Aug 18, 2018

Member

You could throw warning if these variable names are found in the autolev code. I recommend opening an issue for this feature request.

Show outdated Hide outdated doc/src/modules/physics/mechanics/autolev_parser.rst
b = a.subs({q1:30*0.0174533})
# b = a.subs({q1:np.deg2rad(30)}
------------------------------------------------------------------------

This comment has been minimized.

@moorepants

moorepants Aug 18, 2018

Member

What are these lines for? Is this valid restructuredtext?

@moorepants

moorepants Aug 18, 2018

Member

What are these lines for? Is this valid restructuredtext?

This comment has been minimized.

@NikhilPappu

NikhilPappu Aug 18, 2018

Contributor

It seems to be valid restructuredText. These lines render well and separate the different sections nicely.

@NikhilPappu

NikhilPappu Aug 18, 2018

Contributor

It seems to be valid restructuredText. These lines render well and separate the different sections nicely.

Show outdated Hide outdated doc/src/modules/physics/mechanics/autolev_parser.rst
all occurrences of it in the Kane's equations. For example have a look at
line 10 of this `spring damper example <https://github.com/sympy/sympy/blob/master/sympy/parsing/autolev/test_examples/mass_spring_damper.py>`_.
This equation is used in forming the Kane's equations so we need to
change ``me.dynamicsymbols._t`` to ``me.dynamicsymbols('t')`` in this case.

This comment has been minimized.

@moorepants

moorepants Aug 18, 2018

Member

I don't understand what this is all about. You should never create a dynamicsymbols('t'). Does autolev allow t as an arbitrary variable?

@moorepants

moorepants Aug 18, 2018

Member

I don't understand what this is all about. You should never create a dynamicsymbols('t'). Does autolev allow t as an arbitrary variable?

This comment has been minimized.

@NikhilPappu

NikhilPappu Aug 18, 2018

Contributor

The main problem is that if Autolev finds the variable T in an equation it substitutes the time instance for it during numerical substitution.
In PyDy, we have to explicitly pass in specifieds in the System class and only these specifieds are substituted for with time instances.
This problem can be fixed if PyDy's System class can substitute the time instances for occurences of dynamicsymbols._t.

@NikhilPappu

NikhilPappu Aug 18, 2018

Contributor

The main problem is that if Autolev finds the variable T in an equation it substitutes the time instance for it during numerical substitution.
In PyDy, we have to explicitly pass in specifieds in the System class and only these specifieds are substituted for with time instances.
This problem can be fixed if PyDy's System class can substitute the time instances for occurences of dynamicsymbols._t.

Show outdated Hide outdated doc/src/modules/physics/mechanics/autolev_parser.rst
a = sm.solve(zero,x,y)[x]*2 + sm.solve(zero,x,y)[y]
The indexing like ``[x]`` and ``[y]`` doesn't always work so you might want to
look at the underlying dictionary that solve returns and index it correctly.

This comment has been minimized.

@moorepants

moorepants Aug 18, 2018

Member

Is autolev's strictly a linear solver? If so, maybe we should make use of sympy's newer linsolve().

@moorepants

moorepants Aug 18, 2018

Member

Is autolev's strictly a linear solver? If so, maybe we should make use of sympy's newer linsolve().

This comment has been minimized.

@NikhilPappu

NikhilPappu Aug 19, 2018

Contributor

autolev's solve isn't just a linear solver so I have used solve in this case. I have used linsolve for autolev's CODE Algebraic() command.

@NikhilPappu

NikhilPappu Aug 19, 2018

Contributor

autolev's solve isn't just a linear solver so I have used solve in this case. I have used linsolve for autolev's CODE Algebraic() command.

@moorepants moorepants merged commit 28dd0d3 into sympy:master Aug 21, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
sympy-bot/release-notes The release notes look OK
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment