8
\$\begingroup\$

here is my first project : I have made a double pendulum animation with tkinter on Python. Can you give me some feedback on what can be improved ? Thanks !

Code :

# General imports
import tkinter as tk
import random
import math as m

# Parameters
G = 9.81


class Pendulum():
    def __init__(self, theta: float, theta_dot: float,
                 mass: float, length: float,
                 width: int = 3):
        """Creates a Pendulum with a given position, velocity, length and mass.
        width represent the width of the rope of the pendulum.
        The size of the pendulum is proportional to its mass."""

        self.theta = theta
        self.theta_dot = theta_dot
        self.mass = mass
        self.length = length
        self.width = width


class App(tk.Tk):
    def __init__(self,
                 pendulum_1: Pendulum, pendulum_2: Pendulum,
                 width: int = 600, height: int = 600,
                 offset_width: int = 300, offset_height: int = 120,
                 dt: float = 0.05):
        """Initialize the widget for the double pendulum animation.

        offset_width and offset_height represent the x and y offsets from the
        top left corner of the canvas to place the first pendulum."""

        # Setting attributes
        self.width = width
        self.height = height
        self.offset_width = offset_width
        self.offset_height = offset_height
        self.dt = dt
        self.pendulum_1 = pendulum_1
        self.pendulum_2 = pendulum_2
        self.trace_coords = []

        # Setting canvas widget
        tk.Tk.__init__(self)
        self.title("Double Pendulum")
        self.canvas = tk.Canvas(self,
                                width=self.width, height=self.height)
        self.canvas.pack(side="top")

        # Action
        self.after(1, self.draw_frame)

    def update_pendulums_positions(self):
        """Update the angle positions and velocities of the two pendulums"""

        # Dealing with the first pendulum equation of motion
        num_1 = -G * (2 * self.pendulum_1.mass + self.pendulum_2.mass)
        num_1 *= m.sin(self.pendulum_1.theta)

        num_2 = -self.pendulum_2.mass * G
        num_2 *= m.sin(
            self.pendulum_1.theta -
            2 * self.pendulum_2.theta
        )

        num_3 = -2 * m.sin(self.pendulum_1.theta-self.pendulum_2.theta)
        num_3 *= self.pendulum_2.mass
        num_3 *= (
            self.pendulum_2.theta_dot**2 * self.pendulum_2.length +
            self.pendulum_1.theta_dot**2 * self.pendulum_1.length *
            m.cos(
                self.pendulum_1.theta -
                self.pendulum_2.theta
            )
        )

        denom_1 = self.pendulum_1.length * (
            2 * self.pendulum_1.mass +
            self.pendulum_2.mass -
            self.pendulum_2.mass *
            m.cos(
                2 * self.pendulum_1.theta -
                2 * self.pendulum_2.theta
            )
        )

        # Dealing with the second pendulum equation of motion

        num_4 = 2 * m.sin(self.pendulum_1.theta - self.pendulum_2.theta)

        num_5 = (
            self.pendulum_1.theta_dot**2 *
            self.pendulum_1.length *
            (self.pendulum_1.mass + self.pendulum_2.mass)
        )

        num_6 = G * (self.pendulum_1.mass + self.pendulum_2.mass)
        num_6 *= m.cos(self.pendulum_1.theta)

        num_7 = self.pendulum_2.theta_dot**2 * self.pendulum_2.length
        num_7 *= self.pendulum_2.mass * m.cos(
            self.pendulum_1.theta -
            self.pendulum_2.theta
        )

        denom_2 = self.pendulum_2.length * (
            2 * self.pendulum_1.mass +
            self.pendulum_2.mass -
            self.pendulum_2.mass *
            m.cos(
                2 * self.pendulum_1.theta -
                2 * self.pendulum_2.theta
            )
        )

        # Compute the accelerations
        theta1_dotdot = (num_1 + num_2 + num_3) / denom_1
        theta2_dotdot = (num_4*(num_5+num_6+num_7)) / denom_2

        # Update the velocities and positions
        self.pendulum_1.theta_dot += theta1_dotdot * self.dt
        self.pendulum_1.theta += self.pendulum_1.theta_dot * self.dt
        self.pendulum_2.theta_dot += theta2_dotdot * self.dt
        self.pendulum_2.theta += self.pendulum_2.theta_dot * self.dt

    def draw_pendulums(self):
        """Draw the two pendulums and the trace"""

        # Cartesian coordinates
        x1 = self.pendulum_1.length * m.sin(self.pendulum_1.theta)
        y1 = self.pendulum_1.length * m.cos(self.pendulum_1.theta)

        x2 = x1 + self.pendulum_2.length * m.sin(self.pendulum_2.theta)
        y2 = y1 + self.pendulum_2.length * m.cos(self.pendulum_2.theta)

        # Update the trace of the second pendulum
        self.trace_coords.append(
            (
                self.offset_width + x2,
                self.offset_height + y2,
                self.offset_width + x2,
                self.offset_height + y2
            )
        )

        # Draw the trace
        self.canvas.create_line(self.trace_coords, fill='black', tag='trace')

        # Draw the first pendulum
        self.canvas.create_line(
            self.offset_width, self.offset_height,
            self.offset_width + x1, self.offset_height + y1,
            width=self.pendulum_1.width, fill='pink', tags='pendulum'
        )
        self.canvas.create_oval(
            self.offset_width - self.pendulum_1.mass + x1,
            self.offset_height - self.pendulum_1.mass + y1,
            self.offset_width + self.pendulum_1.mass + x1,
            self.offset_height + self.pendulum_1.mass + y1,
            fill='pink', outline='pink', tags='pendulum'
        )

        # Draw the second pendulum
        self.canvas.create_line(
            self.offset_width + x1, self.offset_height + y1,
            self.offset_width + x2, self.offset_height + y2,
            width=self.pendulum_2.width, fill='pink', tags='pendulum'
        )
        self.canvas.create_oval(
            self.offset_width - self.pendulum_2.mass + x2,
            self.offset_height - self.pendulum_2.mass + y2,
            self.offset_width + self.pendulum_2.mass + x2,
            self.offset_height + self.pendulum_2.mass + y2,
            fill='pink', outline='pink', tags='pendulum'
        )

    def draw_frame(self):
        """Draw the current frame"""

        # Delete objects on the canvas to redraw
        self.canvas.delete('trace')
        self.canvas.delete('pendulum')

        # Update the positions and draw the frame
        self.update_pendulums_positions()
        self.draw_pendulums()

        # Repeat
        self.after(1, self.draw_frame)


if __name__ == '__main__':

    # Initialization of the two pendulums
    theta1 = random.random() * 2 * m.pi
    theta2 = random.random() * 2 * m.pi

    pendulum_1_parameters = {
        "theta": theta1,
        "theta_dot": 0,
        "mass": 10,
        "length": 100,
        "width": 3
    }
    pendulum_2_parameters = {
        "theta": theta2,
        "theta_dot": 0,
        "mass": 10,
        "length": 100,
        "width": 3
    }

    pendulum_1 = Pendulum(**pendulum_1_parameters)
    pendulum_2 = Pendulum(**pendulum_2_parameters)

    # Run the animation
    animation_parameters = {
        "pendulum_1": pendulum_1,
        "pendulum_2": pendulum_2,
        "width": 600,
        "height": 600,
        "offset_width": 300,
        "offset_height": 150,
        "dt": 0.05
    }
    app = App(**animation_parameters)
    app.mainloop()
\$\endgroup\$
1
2
\$\begingroup\$

It's hypnotic to watch them balacing!

You assume the delta time is always 0,05sec. For a better simulation you should retrive real delta time. In terms of code quality, in method update_pendulums_positions, your code will be heavily more readable if you use explicit variable names instead of "num_" something. Yeah, it's physics computation, but still, all computation has a meaning. Your code won't run slower if variable names are longer! (yes, already heard a coworker saying that for C++)

You should also refactor some pieces, like:

  • draw_pendulums: the create_line and create_oval are the same, just take pendulum as a parameter of a function

  • computation of "denom" (denominator?): only the length change between the two of them

  • updating velocities and positions: the two lines for each pendulum are the same

  • cardinal coordinates, just add an (x,y) offset as a parameter to handle computation of the second pendulum depending on the coordinates of the first one

  • num_3 and num_7: if I'm right, those two can be written as

num_3 = -2 * m.sin(self.pendulum_1.theta - self.pendulum_2.theta)
num_3 *= self.pendulum_2.mass * self.pendulum_2.theta_dot**2 * self.pendulum_2.length
num_3 *= 1 + m.cos(self.pendulum_1.theta - self.pendulum_2.theta)

num_7 = self.pendulum_2.mass * self.pendulum_2.theta_dot**2 * self.pendulum_2.length
num_7 *= m.cos(self.pendulum_1.theta - self.pendulum_2.theta)

You can see that some computations are done twice. Use intermediate variable to do those once. Your app is not heavy, but that would help to make something more scalable by optimizing your code.

TL,DR: DRY and Self-documented code (even though writing comments is a rule, it can't hurt to have a 'human-readable' code)

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Instead of full answer, let me expand on the last line of @VincentRG. Looking at your comments, draw_frame() has the comment Draw the current frame. You could rename it to draw_current_frame() and throw away the comment. Comments cause future maintenance costs, like any other code. \$\endgroup\$ – Charles Merriam Feb 2 '20 at 2:26

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.