patternpythonMinor
4-Bar Mechanism Generation
Viewed 0 times
generationbarmechanism
Problem
For a school project, I will design and prototype a bicycle brake that uses a four-bar linkage to accomplish its goal.
My Python 3 code does this mechanism generation, and implements methods to remove mechanisms I don't think will work well to hopefully give me a good final mechanism to construct. Overall I wrote this up quite quickly, hence its messiness. I'm hoping to dramatically clean up the code and improve efficiency. Any suggestions on where to begin?
Background
(This section isn't critical to review the code, but I thought would be nice to include)
To create the mechanism, I need to define a set of three "precision positions" on the coupler curve, which are points the coupler point of the four-bar will pass through.
Since the points I define are just approximate guesses what I think the path should be (approximate linear motion), I loop over a small range for each coordinate I define.
I then also choose the driver rotation between PP1 and PP2 (\$ \beta_2 \$) and PP1 and PP3 (\$ \beta_3 \$). The \$ \alpha \$'s are solved from the given precision positions (the couplers rotation), as well as the \$ \delta \$'s (the translational movement of the coupler point). The following picture illustrates this:
The following system of equations is solved to obtain a 2 dyads (shown above, cut the mechanism in half between the coupler point, the two links on each side is a dyad). The two dyads together form the 4-bar.
$$\vec{W_A}(e^{i\beta_{2A}} - 1) + \vec{Z_A}(e^{i\alpha_{2A}} - 1) = \delta_2$$
$$\vec{W_A}(e^{i\beta_{3A}} - 1) + \vec{Z_A}(e^{i\alpha_{3A}} - 1) = \delta_3$$
$$\vec{W_B}(e^{i\beta_{2B}} - 1) + \vec{Z_B}(e^{i\beta_{2A}} - 1) = \delta_2$$
$$\vec{W_B}(e^{i\beta_{3B}} - 1) + \vec{Z_B}(e^{i\beta_{3A}} - 1) = \delta_3$$
gen.py:
```
import numpy as np
import matplotlib.pyplot as plt
import math
from filter import links_to_joints, filter_length, filter_orientation, filter_configuration, filter_mechanical_advantage
from plot import plot_four_bar_
My Python 3 code does this mechanism generation, and implements methods to remove mechanisms I don't think will work well to hopefully give me a good final mechanism to construct. Overall I wrote this up quite quickly, hence its messiness. I'm hoping to dramatically clean up the code and improve efficiency. Any suggestions on where to begin?
Background
(This section isn't critical to review the code, but I thought would be nice to include)
To create the mechanism, I need to define a set of three "precision positions" on the coupler curve, which are points the coupler point of the four-bar will pass through.
Since the points I define are just approximate guesses what I think the path should be (approximate linear motion), I loop over a small range for each coordinate I define.
I then also choose the driver rotation between PP1 and PP2 (\$ \beta_2 \$) and PP1 and PP3 (\$ \beta_3 \$). The \$ \alpha \$'s are solved from the given precision positions (the couplers rotation), as well as the \$ \delta \$'s (the translational movement of the coupler point). The following picture illustrates this:
The following system of equations is solved to obtain a 2 dyads (shown above, cut the mechanism in half between the coupler point, the two links on each side is a dyad). The two dyads together form the 4-bar.
$$\vec{W_A}(e^{i\beta_{2A}} - 1) + \vec{Z_A}(e^{i\alpha_{2A}} - 1) = \delta_2$$
$$\vec{W_A}(e^{i\beta_{3A}} - 1) + \vec{Z_A}(e^{i\alpha_{3A}} - 1) = \delta_3$$
$$\vec{W_B}(e^{i\beta_{2B}} - 1) + \vec{Z_B}(e^{i\beta_{2A}} - 1) = \delta_2$$
$$\vec{W_B}(e^{i\beta_{3B}} - 1) + \vec{Z_B}(e^{i\beta_{3A}} - 1) = \delta_3$$
gen.py:
```
import numpy as np
import matplotlib.pyplot as plt
import math
from filter import links_to_joints, filter_length, filter_orientation, filter_configuration, filter_mechanical_advantage
from plot import plot_four_bar_
Solution
check_lengthsAvoid repetition
When the same line of code is repeated many many times the information gets lost in the noise and it is not obvious at first glance:
not (np.abs(four_bar_set[a][b]) - np.abs(four_bar_set[c][d])) < 1e-10
not (np.abs(four_bar_set[a][b]) - np.abs(four_bar_set[c][d])) < 1e-10
...or
In this lines of code only
a,b,c and d change, all the rest is equal, how can we do better?The solution is to make a data structure containing the variable data:
values = [ # <- Give better name
(0, 1, 1, 1),
(0, 2, 1, 3),
...
]And the using the
any built-in that is the same as reduceing or over a list.if (any( not (np.abs(four_bar_set[a][b]) - np.abs(four_bar_set[c][d])) < 1e-10 ) for a, b, c, d in values)):
# HandleWe could even go a step further and define an helper function:
def four_bar_set_error_inside_limit(limit, a, b, c, d):
return (np.abs(four_bar_set[a][b]) - np.abs(four_bar_set[c][d])) < 1e-10and write:
if (all( four_bar_set_error_inside_limit(limit, a, b, c, d) ) for a,b,c,d in values):
# HandlePlease note that this way the code is immediately clear, also because I removed the double negation of
any and not and used just all.Avoid printing from inside a function
print('ERROR!')
returnThis way of handling things is one-off, non-standard and hard to reuse.
The caller function should call this function to know if a certain condition is met (which condition exactly? Without docstring and with such a general name the user of this function is forced to read the code to understand, that should not happen), then you should let him free to handle this result however he likes, maybe logging a message, maybe crashing, maybe trying another way of calculation.
So in my opinion you should just return the value, so:
Final version
def four_bar_set_error_inside_limit(tolerance, a, b, c, d):
return (np.abs(four_bar_set[a][b]) - np.abs(four_bar_set[c][d])) < tolerance
def all_lengths_deltas_below_error(four_bar_set, tolerance):
values = [...]
return all(four_bar_set_error_inside_limit(tolerance, a, b, c, d)
for a, b, c, d in values)Also note that this final version has a
tolerance parameter that you can easily pass from the outside while the previous version had the value 1e-10 repeated 8 times and changing it would be time consuming and prone to error forgetting one.A final improvement would be to add a doctest and a small docstring explaining exactly what a
four_bar_set is in real life and how it is represented, and why it is important to check that this property holds true.filter_lengthAvoid an
out parameterdef filter_length(solutions, filtered_solutions, four_bar_list):
for link in four_bar_list[0]:
# remove any solutions with links greater than 50 mm or less than 10mm
if np.abs(link) 80:
return
else:
filtered_solutions.append(four_bar_list)In this function
filtered_solutions is an out parameter, that is a parameter that is passed into a function for the sole purpose of being modified to contain the output of the function.This is non-standard in modern Python and there is a much simpler way to handle this, the
yield keyword:def filter_length(solutions, filtered_solutions, four_bar_list):
for link in four_bar_list[0]:
# remove any solutions with links greater than 50 mm or less than 10mm
if not (20 < np.abs(link) < 80):
return
yield four_bar_listWrong comments are worse than no comments
# remove any solutions with links greater than 50 mm or less than 10mm
if np.abs(link) 80:
returnThe comment states 50 and 10 while the code states 20 and 80 so something is clearly wrong.
Maybe you are using different units of measure between the comment and the code, or maybe you changed your mind halfway between writing your comment and the code, or maybe you changed the code and forgotten the comment.
Anyhow this is confusing, also because no link to an original source or material is given in order to discern which between the comment and the code is correct.
This is another example of why comments stating the obvious and repeating the code should not be written in the first place (https://en.wikipedia.org/wiki/Single_source_of_truth and https://en.wikipedia.org/wiki/Dont_repeat_yourself principles).
Code Snippets
not (np.abs(four_bar_set[a][b]) - np.abs(four_bar_set[c][d])) < 1e-10
not (np.abs(four_bar_set[a][b]) - np.abs(four_bar_set[c][d])) < 1e-10
...values = [ # <- Give better name
(0, 1, 1, 1),
(0, 2, 1, 3),
...
]if (any( not (np.abs(four_bar_set[a][b]) - np.abs(four_bar_set[c][d])) < 1e-10 ) for a, b, c, d in values)):
# Handledef four_bar_set_error_inside_limit(limit, a, b, c, d):
return (np.abs(four_bar_set[a][b]) - np.abs(four_bar_set[c][d])) < 1e-10if (all( four_bar_set_error_inside_limit(limit, a, b, c, d) ) for a,b,c,d in values):
# HandleContext
StackExchange Code Review Q#159776, answer score: 2
Revisions (0)
No revisions yet.