Add algorithm for N-body simulation - retry #4298
Conversation
Click here to look at the relevant links
|
| body.update_position(delta_time * self.time_factor) | ||
|
|
||
|
|
||
| def plot( |
algorithms-keeper
bot
Mar 28, 2021
As there is no test file in this pull request nor any test function or class in the file other/n_body_simulation.py, please provide doctest for the function plot
algobytewise
Mar 28, 2021
Author
Contributor
No doctest provided since this function does not have a return value, it just plots the result of the algorithm.
| ) | ||
|
|
||
| # Function called once at the start of the animation | ||
| def init() -> list[patches.Circle]: |
algorithms-keeper
bot
Mar 28, 2021
As there is no test file in this pull request nor any test function or class in the file other/n_body_simulation.py, please provide doctest for the function init
cclauss
Mar 28, 2021
Member
You mean that because it is an inner function it cannot contain bugs?
It needs tests.
algobytewise
Mar 28, 2021
Author
Contributor
I'm not sure that it's possible to write a doctest for an inner function, for example, see https://stackoverflow.com/questions/2136910/can-i-unit-test-an-inner-function-in-python or https://bugs.python.org/issue1650090 . I could define the 2 functions outside the plot-function and add doctests. But that would be a little unintuitive, since they are just part of the plotting and not of the algorithm proper. But my experience with doctest is limited so I'm not sure what the best approach here is.
cclauss
Mar 28, 2021
•
Member
I do not see the advantage of making this an inner function. Just make it a normal function with proper tests.
algobytewise
Mar 28, 2021
Author
Contributor
I ran into some problems while trying to define init & update outside the scope of plot, since these functions make use of patches, which is defined inside plot. Something like this seems to be the normal procedure for using matplotlib animation, for example, see line in https://jakevdp.github.io/blog/2012/08/18/matplotlib-animation-tutorial/ .
I don't think that we can pass patches as an argument to these functions since animation.FuncAnimation uses them as callbacks. It might be possible to solve this problem through currying, but that would make it needlessly complicated and may defy the attempt to avoid using inner functions. But maybe there is an easier solution that I'm missing.
algobytewise
Mar 28, 2021
Author
Contributor
Another approach would be to define patches (& possibly other local variables needed) globally and then import them into the functions using the global-keyword. It should work but it's not pretty.
cclauss
Mar 28, 2021
•
Member
If you want to keep the inner functions then the outer function should be heavily tested. Let's put all calculation in a separate function from the plotting function.
Make the function that plots as small as possible... That is, make one well-tested function that calculates the results and another smaller, untested function that performs no calculations but merely plots the results.
| return patches | ||
|
|
||
| # Function called at each step of the animation | ||
| def update(frame: int) -> list[patches.Circle]: |
algorithms-keeper
bot
Mar 28, 2021
As there is no test file in this pull request nor any test function or class in the file other/n_body_simulation.py, please provide doctest for the function update
| @@ -0,0 +1,262 @@ | |||
| """ | |||
| In physics and astronomy, a gravitational N-body simulation is a simulation of a | |||
cclauss
Mar 28, 2021
Member
Should we have physics and/or (even better) astronomy directory so that we do not grow the other directory.
algobytewise
Mar 28, 2021
Author
Contributor
If we make it physics, we could also include the files from electronics and remove one root-directory. But in any case, I think physics is the better choice since this is not specifically about celestial objects.
cclauss
Mar 28, 2021
Member
Let's add physics with this algorithm but let's not migrate electronics into it.
| plt.show() | ||
|
|
||
|
|
||
| if __name__ == "__main__": |
cclauss
Mar 28, 2021
Member
Please break this long collection of code into three separate functions.
| if __name__ == "__main__": | |
| if __name__ == "__main__": | |
| example_1() | |
| example_2() | |
| example_3() |
| patches = [] | ||
| for body in body_system.bodies: | ||
| patches.append( | ||
| plt.Circle((body.position_x, body.position_y), body.size, fc=body.color) | ||
| ) |
| >>> body = Body(0.,0.,1.,0.) | ||
| >>> body.update_position(1.) | ||
| >>> body.position_x | ||
| 1.0 |
| >>> body_system = BodySystem([Body(0,0,0,0), Body(10,0,0,0)]) | ||
| >>> body_system.update_system(1) | ||
| >>> body_system.bodies[0].position_x |
| patches = [] | ||
| for body in body_system.bodies: | ||
| patches.append( | ||
| plt.Circle((body.position_x, body.position_y), body.size, fc=body.color) | ||
| ) |
cclauss
Mar 29, 2021
Member
| patches = [] | |
| for body in body_system.bodies: | |
| patches.append( | |
| plt.Circle((body.position_x, body.position_y), body.size, fc=body.color) | |
| ) | |
| patches = [ | |
| plt.Circle((body.position_x, body.position_y), body.size, fc=body.color) | |
| for body in body_system.bodies | |
| ] |
Click here to look at the relevant links
|
| body.update_position(delta_time * self.time_factor) | ||
|
|
||
|
|
||
| def plot( |
algorithms-keeper
bot
Mar 29, 2021
As there is no test file in this pull request nor any test function or class in the file physics/n_body_simulation.py, please provide doctest for the function plot
| ) | ||
|
|
||
| # Function called once at the start of the animation | ||
| def init() -> list[patches.Circle]: |
algorithms-keeper
bot
Mar 29, 2021
As there is no test file in this pull request nor any test function or class in the file physics/n_body_simulation.py, please provide doctest for the function init
| return patches | ||
|
|
||
| # Function called at each step of the animation | ||
| def update(frame: int) -> list[patches.Circle]: |
algorithms-keeper
bot
Mar 29, 2021
As there is no test file in this pull request nor any test function or class in the file physics/n_body_simulation.py, please provide doctest for the function update
Click here to look at the relevant links
|
Click here to look at the relevant links
|
Click here to look at the relevant links
|
|
Several additional doctests were added for the methods of |
Click here to look at the relevant links
|
|
|
||
| class Body: | ||
| def __init__( | ||
| self: Body, |
dhruvmanila
Apr 4, 2021
Member
self should not be given a type hint. It is evaluated using dynamic dispatch during runtime. Please remove it from other places as well.
algorithms-keeper
bot
added
awaiting reviews
and removed
require tests
awaiting changes
labels
Click here to look at the relevant links
|
| self.color = color | ||
|
|
||
| @property | ||
| def position(self) tuple[float, float]: |
algorithms-keeper
bot
Apr 4, 2021
An error occured while parsing the file: physics/n_body_simulation.py
Traceback (most recent call last):
File "/app/.heroku/python/lib/python3.8/site-packages/libcst/_parser/base_parser.py", line 152, in _add_token
plan = stack[-1].dfa.transitions[transition]
KeyError: TokenType(NAME)
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/app/algorithms_keeper/parser/python_parser.py", line 145, in parse
reports = lint_file(
libcst._exceptions.ParserSyntaxError: Syntax Error @ 47:24.
Incomplete input. Encountered 'tuple', but expected '->', or ':'.
def position(self) tuple[float, float]:
^
Click here to look at the relevant links
|
| return self.position_x, self.position_y | ||
|
|
||
| @property | ||
| def velocity(self) tuple[float, float]: |
algorithms-keeper
bot
Apr 4, 2021
An error occured while parsing the file: physics/n_body_simulation.py
Traceback (most recent call last):
File "/app/.heroku/python/lib/python3.8/site-packages/libcst/_parser/base_parser.py", line 152, in _add_token
plan = stack[-1].dfa.transitions[transition]
KeyError: TokenType(NAME)
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/app/algorithms_keeper/parser/python_parser.py", line 145, in parse
reports = lint_file(
libcst._exceptions.ParserSyntaxError: Syntax Error @ 51:24.
Incomplete input. Encountered 'tuple', but expected '->', or ':'.
def velocity(self) tuple[float, float]:
^
Click here to look at the relevant links
|
| self.color = color | ||
|
|
||
| @property | ||
| def position(self) -> tuple[float, float]: |
algorithms-keeper
bot
Apr 4, 2021
As there is no test file in this pull request nor any test function or class in the file physics/n_body_simulation.py, please provide doctest for the function position
| return self.position_x, self.position_y | ||
|
|
||
| @property | ||
| def velocity(self) -> tuple[float, float]: |
algorithms-keeper
bot
Apr 4, 2021
As there is no test file in this pull request nor any test function or class in the file physics/n_body_simulation.py, please provide doctest for the function velocity
| self.time_factor = time_factor | ||
| self.softening_factor = softening_factor | ||
|
|
||
| def __len__() -> int: |
algorithms-keeper
bot
Apr 4, 2021
As there is no test file in this pull request nor any test function or class in the file physics/n_body_simulation.py, please provide doctest for the function __len__
| patch.center = (body.position_x, body.position_y) | ||
|
|
||
|
|
||
| def plot( |
algorithms-keeper
bot
Apr 4, 2021
As there is no test file in this pull request nor any test function or class in the file physics/n_body_simulation.py, please provide doctest for the function plot
| ax.add_patch(patch) | ||
|
|
||
| # Function called at each step of the animation | ||
| def update(frame: int) -> list[plt.Circle]: |
algorithms-keeper
bot
Apr 4, 2021
As there is no test file in this pull request nor any test function or class in the file physics/n_body_simulation.py, please provide doctest for the function update
| return BodySystem(bodies1, time_factor=3) | ||
|
|
||
|
|
||
| def example_2() -> BodySystem: |
algorithms-keeper
bot
Apr 4, 2021
As there is no test file in this pull request nor any test function or class in the file physics/n_body_simulation.py, please provide doctest for the function example_2
| return BodySystem([earth, moon], gravitation_constant, time_factor=1000000) | ||
|
|
||
|
|
||
| def example_3() -> BodySystem: |
algorithms-keeper
bot
Apr 4, 2021
As there is no test file in this pull request nor any test function or class in the file physics/n_body_simulation.py, please provide doctest for the function example_3
Click here to look at the relevant links
|
| self.color = color | ||
|
|
||
| @property | ||
| def position(self) -> tuple[float, float]: |
algorithms-keeper
bot
Apr 4, 2021
As there is no test file in this pull request nor any test function or class in the file physics/n_body_simulation.py, please provide doctest for the function position
| return self.position_x, self.position_y | ||
|
|
||
| @property | ||
| def velocity(self) -> tuple[float, float]: |
algorithms-keeper
bot
Apr 4, 2021
As there is no test file in this pull request nor any test function or class in the file physics/n_body_simulation.py, please provide doctest for the function velocity
| self.softening_factor = softening_factor | ||
|
|
||
| def __len__() -> int: | ||
| return len(self.bodies) |
algorithms-keeper
bot
Apr 4, 2021
As there is no test file in this pull request nor any test function or class in the file physics/n_body_simulation.py, please provide doctest for the function __len__
|
|
||
|
|
||
| def plot( | ||
| title: str, |
algorithms-keeper
bot
Apr 4, 2021
As there is no test file in this pull request nor any test function or class in the file physics/n_body_simulation.py, please provide doctest for the function plot
|
|
||
| # Function called at each step of the animation | ||
| def update(frame: int) -> list[plt.Circle]: | ||
| update_step(body_system, DELTA_TIME, patches) |
algorithms-keeper
bot
Apr 4, 2021
As there is no test file in this pull request nor any test function or class in the file physics/n_body_simulation.py, please provide doctest for the function update
|
|
||
|
|
||
| def example_2() -> BodySystem: | ||
| """ |
algorithms-keeper
bot
Apr 4, 2021
As there is no test file in this pull request nor any test function or class in the file physics/n_body_simulation.py, please provide doctest for the function example_2
|
|
||
|
|
||
| def example_3() -> BodySystem: | ||
| """ |
algorithms-keeper
bot
Apr 4, 2021
As there is no test file in this pull request nor any test function or class in the file physics/n_body_simulation.py, please provide doctest for the function example_3
Click here to look at the relevant links
|
| self.color = color | ||
|
|
||
| @property | ||
| def position(self) -> tuple[float, float]: |
algorithms-keeper
bot
Apr 4, 2021
As there is no test file in this pull request nor any test function or class in the file physics/n_body_simulation.py, please provide doctest for the function position
| return self.position_x, self.position_y | ||
|
|
||
| @property | ||
| def velocity(self) -> tuple[float, float]: |
algorithms-keeper
bot
Apr 4, 2021
As there is no test file in this pull request nor any test function or class in the file physics/n_body_simulation.py, please provide doctest for the function velocity
| self.time_factor = time_factor | ||
| self.softening_factor = softening_factor | ||
|
|
||
| def __len__(self) -> int: |
algorithms-keeper
bot
Apr 4, 2021
As there is no test file in this pull request nor any test function or class in the file physics/n_body_simulation.py, please provide doctest for the function __len__
| patch.center = (body.position_x, body.position_y) | ||
|
|
||
|
|
||
| def plot( |
algorithms-keeper
bot
Apr 4, 2021
As there is no test file in this pull request nor any test function or class in the file physics/n_body_simulation.py, please provide doctest for the function plot
| ax.add_patch(patch) | ||
|
|
||
| # Function called at each step of the animation | ||
| def update(frame: int) -> list[plt.Circle]: |
algorithms-keeper
bot
Apr 4, 2021
As there is no test file in this pull request nor any test function or class in the file physics/n_body_simulation.py, please provide doctest for the function update
| return BodySystem(bodies1, time_factor=3) | ||
|
|
||
|
|
||
| def example_2() -> BodySystem: |
algorithms-keeper
bot
Apr 4, 2021
As there is no test file in this pull request nor any test function or class in the file physics/n_body_simulation.py, please provide doctest for the function example_2
| return BodySystem([earth, moon], gravitation_constant, time_factor=1000000) | ||
|
|
||
|
|
||
| def example_3() -> BodySystem: |
algorithms-keeper
bot
Apr 4, 2021
As there is no test file in this pull request nor any test function or class in the file physics/n_body_simulation.py, please provide doctest for the function example_3
Click here to look at the relevant links
|
| self.color = color | ||
|
|
||
| @property | ||
| def position(self) -> tuple[float, float]: |
algorithms-keeper
bot
Apr 4, 2021
As there is no test file in this pull request nor any test function or class in the file physics/n_body_simulation.py, please provide doctest for the function position
| return self.position_x, self.position_y | ||
|
|
||
| @property | ||
| def velocity(self) -> tuple[float, float]: |
algorithms-keeper
bot
Apr 4, 2021
As there is no test file in this pull request nor any test function or class in the file physics/n_body_simulation.py, please provide doctest for the function velocity
| self.time_factor = time_factor | ||
| self.softening_factor = softening_factor | ||
|
|
||
| def __len__(self) -> int: |
algorithms-keeper
bot
Apr 4, 2021
As there is no test file in this pull request nor any test function or class in the file physics/n_body_simulation.py, please provide doctest for the function __len__
| patch.center = (body.position_x, body.position_y) | ||
|
|
||
|
|
||
| def plot( |
algorithms-keeper
bot
Apr 4, 2021
As there is no test file in this pull request nor any test function or class in the file physics/n_body_simulation.py, please provide doctest for the function plot
| ax.add_patch(patch) | ||
|
|
||
| # Function called at each step of the animation | ||
| def update(frame: int) -> list[plt.Circle]: |
algorithms-keeper
bot
Apr 4, 2021
As there is no test file in this pull request nor any test function or class in the file physics/n_body_simulation.py, please provide doctest for the function update
| return BodySystem(bodies1, time_factor=3) | ||
|
|
||
|
|
||
| def example_2() -> BodySystem: |
algorithms-keeper
bot
Apr 4, 2021
As there is no test file in this pull request nor any test function or class in the file physics/n_body_simulation.py, please provide doctest for the function example_2
| return BodySystem([earth, moon], gravitation_constant, time_factor=1000000) | ||
|
|
||
|
|
||
| def example_3() -> BodySystem: |
algorithms-keeper
bot
Apr 4, 2021
As there is no test file in this pull request nor any test function or class in the file physics/n_body_simulation.py, please provide doctest for the function example_3
|
I will create a follow up PR to add tests to examples. |
|
Thanks for helping with all the changes. It's definitely more elegantly handled this way. |
* add n_body_simulation.py
* updating DIRECTORY.md
* Rename other/n_body_simulation.py to physics/n_body_simulation.py
* updating DIRECTORY.md
* Update build.yml
* refactor examples & add doctests
* removed type-hints from self-parameter
* Apply suggestions from code review
* Update physics/n_body_simulation.py
* Update physics/n_body_simulation.py
* Update physics/n_body_simulation.py
* Don't forget self
* Fix velocity
Co-authored-by: github-actions <${GITHUB_ACTOR}@users.noreply.github.com>
Co-authored-by: Dhruv Manilawala <dhruvmanila@gmail.com>
Co-authored-by: Christian Clauss <cclauss@me.com>
Describe your change:
New pull request to replace #4245 since the old pull request ran into a branch conflict after updating. The changes to #4245 include better comments and more descriptive names.
Checklist:
Fixes: #{$ISSUE_NO}.