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

WIP : Mechanics core, Part 1 #2187

Closed
wants to merge 20 commits into from

Conversation

srjoglekar246
Copy link
Member

Classes for MovingRefFrame, Particle and helper functions

GSoC PR1

@srjoglekar246
Copy link
Member Author

global_time = None

@classmethod
def set_time(cls, time_var):
Copy link
Member

Choose a reason for hiding this comment

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

If you have a setter in python you might be doing something wrong. If you have a setter in SymPy you are almost surely doing something wrong. Why isn't this just another argument in the constructor?

Copy link
Member Author

Choose a reason for hiding this comment

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

Wanted to see how having a global time variable turns out. Let the user define a global time variable for the whole class, then all the reference frames initialized from it will use that variable for time only. Unless a time variable is defined, a reference frame won't be initialized (that's why the new) and if a time variable is already defined, a new one cant be defined in that session(that's why the set_time method).
Just an idea to ensure all frames work with same notion of time without breaking immutability.

Copy link
Member

Choose a reason for hiding this comment

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

I still do not understand why do you need __new__ when this error can be raised in __init__ as well (cls == type(self)).

Anyway, this seems like a really strange abuse of classes in python. You are practically creating new classes each time that you use set_time. What will happen for instance if I call set_time a second time? In addition, set_time will be visible to the instances of the class as well - this is sure to cause confusion.

I guess that Prasoon will have some 'World' or 'Space' or something like that class. This seems like a more appropriate place for the time parameter.

Copy link
Member Author

Choose a reason for hiding this comment

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

If you call it for a second time, it will simply raise an error. It will only update the value of global_time if its None initially(I should check if its an instance of Symbol). Because its a class variable, it will not create new instances of MovingRefFrame when its set. While its None, if you try to initialize a frame, it will raise an error, specifying that a time variable has not been defined.
And about the World class, I guess we scraped the idea? And I don't think the design here is violating immutability? If I disallow redefinition of the time variable?

Copy link
Member

Choose a reason for hiding this comment

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

Then what if have to solve two unrelated problems in the same session (that have different symbols for time)?

Copy link
Member Author

Choose a reason for hiding this comment

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

If the problems are unrelated, then there's no way they would interact right? Having the same variable wouldn't affect the working in any ways, the user can even do 'sometemporaryname = MovingRefFrame.global_time' to avoid confusion. But I get the point you are trying to make. I am just not sure putting time in the initialiser is a good idea. IF the user puts different variables for time in the same 'world' it could cause some problems.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the first question I have is on use cases - do we need to support multiple time variables within one physics problem? I've never done that, but @Krastanov, have you?
I have another question though - shouldn't all the associated basis vectors and coordinate functions be functions of time?

Anyways, if we want to allow multiple time variables, I think you could do the following: have an attribute, MovingRefFrame.time=symbols('t'). Each frame should pull from that on creation. If the user wants to then use a different time all of a sudden, they have to set the class's definition of time to None

MovingRefFrame.time=None

and supply the time as a keyword arg to future MovingRefFrame's

N = MovingRefFrame(......., t=symbols('time'))

or an error will be thrown (in the initializer).

@gilbertgede
Copy link
Contributor

I would recommend putting more comments throughout the code. The current mechanics module's code is under-documented, and probably not a good example for how much detail you should cover.

I would also start putting the examples in the docstrings, so it is easier to follow the interface.

Also, for the position/velocity/acceleration/angular velocity/angular acceleration methods - perhaps we should have an external helper function (perhaps even hidden from the user) which does the calculations and caching. I'm not completely sure on this, but I think we might be able to save on computation time, by computing things in one direction only and having that cached?

while i < len(path):
result -= otherframe.express(path[i]._trans_vel)
i += 1
return result
Copy link
Member Author

Choose a reason for hiding this comment

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

This implementation of trans_vel_in should be changed to use time_derivative with pos_vector_in at each step. The current one will give a wrong output in the general case.

@jrioux
Copy link
Member

jrioux commented Jul 9, 2013

SymPy Bot Summary: 🔴 Failed after merging srjoglekar246/framebranch (2019c255fce7584cd08122da2c5e191379a9cf8e) into master (e06036a).
@srjoglekar246: Please fix the test failures.
🔴 PyPy 2.0.0-beta-1; 2.7.3-final-42: fail
🔴 Python 2.7.2-final-0: fail
🔴 Python 3.2.1-final-0: fail
Sphinx 1.1.3: pass
Doc Coverage: unchanged
36.299% of functions have doctests (compared to 36.299% in master)
40.505% of functions are imported into Sphinx (compared to 40.505% in master)

@jrioux
Copy link
Member

jrioux commented Jul 14, 2013

SymPy Bot Summary: 🔴 Failed after merging srjoglekar246/framebranch (160d414213817972e965af048ac048d3734b16c0) into master (40d17b1).
@srjoglekar246: Please fix the test failures.
🔴 PyPy 2.0.0-beta-1; 2.7.3-final-42: fail
🔴 Python 2.7.2-final-0: fail
🔴 Python 3.2.1-final-0: fail
Sphinx 1.1.3: pass
Doc Coverage: unchanged
36.132% of functions have doctests (compared to 36.132% in master)
41.482% of functions are imported into Sphinx (compared to 41.482% in master)

@jrioux
Copy link
Member

jrioux commented Aug 7, 2013

SymPy Bot Summary: 🔴 Failed after merging srjoglekar246/framebranch (2a23a03) into master (0bdb41a).
@srjoglekar246: Please fix the test failures.
🔴 PyPy 2.0.0-beta-1; 2.7.3-final-42: fail
🔴 Python 2.7.2-final-0: fail
🔴 Python 3.2.1-final-0: fail
Sphinx 1.1.3: pass
Doc Coverage: unchanged
36.065% of functions have doctests (compared to 36.065% in master)
41.552% of functions are imported into Sphinx (compared to 41.552% in master)

@asmeurer
Copy link
Member

SymPy Bot Summary: 🔴 Failed after merging srjoglekar246/framebranch (2a23a03) into master (90fc660).
@srjoglekar246: Please fix the test failures.
🔴 Python 2.5.6-final-0: fail
🔴 Python 2.6.8-final-0: fail
🔴 Python 2.7.3-final-0: fail
🔴 PyPy 1.8.0-final-0; 2.7.2-final-42: fail
🔴 Python 3.2.3-final-0: fail
🔴 Sphinx 1.1.3: fail


For velocity- 'position_b', 't' - the position at the value of time t
For acceleration - 'trans_vel_b', 'position_b', 't1', 't2' - the velocity
and position boundary conditions at times t1 and t2 respectively
Copy link
Member

Choose a reason for hiding this comment

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

What are the use cases when you would be specifying the acceleration? F=ma needs "a" and the hard part of dynamics is typically figuring out what "a" is given some arbitrary system of rigid bodies. If you specify what "a" is then you are doing inverse dynamics as opposed to forward dynamics. Can we see some example problems that show why and how you will ever use this?

Copy link
Member Author

Choose a reason for hiding this comment

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

There are no specific examples I can think of right now. But I have dealt with problems where the force on an object was a function of time (not necessarily increasing). In such a case, given the mass, with SymPy's integration abilities, we could be able to find the velocity/position vector as a function of time.
It may not be as common as forward dynamics, but some users may want to have that functionality - of specifying vectorial functions of time, with boundary cases, and getting the motion parameters calculated.

Copy link
Member

Choose a reason for hiding this comment

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

If there are no specific examples that would use the functionality then that means we don't need it. I wouldn't generalize for the sake of generalization. See my latest email on the pydy list. My biggest suggestion here is to create examples problems you want to be able to solve (we've already created a bunch for multibody dynamics) and then design API around the practical things you want to be able to do. This is then much easier for outsiders (i.e. outside the details of your code) to understand and digest.

@hargup
Copy link
Contributor

hargup commented Apr 23, 2014

@srjoglekar246 any update on this?

@moorepants moorepants added the PR: author's turn The PR has been reviewed and the author needs to submit more changes. label Dec 28, 2016
@czgdp1807
Copy link
Member

@sympy/mechanics It looks like a good work. Can this be extended? @srjoglekar246 Any comments from your side?

@czgdp1807
Copy link
Member

I am adding a Please take over label. ping @sympy/mechanics Please let me know if this PR is still needed.

@czgdp1807 czgdp1807 added Please take over and removed PR: author's turn The PR has been reviewed and the author needs to submit more changes. labels Nov 25, 2019
@moorepants
Copy link
Member

This could be useful. I think it was introducing some new objects that combined orientation and translation by attaching a point to a reference frame.

@moorepants
Copy link
Member

We now have a Body class that addresses some of what this PR is. I'm closing this one as partially fixed and also stale.

@moorepants moorepants closed this Jul 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants