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

Remove redundant "dx += accel" #422

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

Zelpuz
Copy link

@Zelpuz Zelpuz commented Jun 21, 2022

In Chapter 1, under "Exercise: The Effect of Acceleration", the solution uses the closed-form solution to the differential equations for acceleration, velocity, and displacement.

def gen_data(x0, dx, count, noise_factor, accel=0.):
    zs = []
    for i in range(count):
        zs.append(x0 + accel * (i**2) / 2 + dx*i + randn()*noise_factor)
        dx += accel
    return zs

However, because this solution was used, there is no need to increment dx; in this solution, dx should just remain as its initial value (which we could call dx0, but I did not change it here).

See #381, #369, and #301. This problem appears to have arisen from the commit associated with #301, where the equation was changed from an iterative method to the closed-form solution, but the dx += accel was not removed.

This does not at all impact the lesson being taught in this section, but it might be confusing for readers if they cannot replicate the solution, especially since the solution uses zero noise.

In Chapter 1, under "Exercise: The Effect of Acceleration", the solution utilizes the closed-form solution to the differential equations for acceleration, velocity, and displacement. Because this solution was used, there is no need to increment dx; in this solution, dx should just remain as its initial value (which we could call dx0, but I didn't change it here).
@smcardle14
Copy link

Note that the text in this section should be reworded to be consistent with this change (especially the paragraph before and after the modified cell).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants