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

Chap15 conv1d and conv2d #132

Open
JohannWO opened this issue Feb 3, 2021 · 3 comments
Open

Chap15 conv1d and conv2d #132

JohannWO opened this issue Feb 3, 2021 · 3 comments

Comments

@JohannWO
Copy link

JohannWO commented Feb 3, 2021

When running the naive Implementation of conv1d and conv2d with example in book then it shows wrong results.

conv1d works for
X = [[1, 3, 2, 4], [5, 6, 1, 3], [1, 2, 0, 2], [3, 4, 3, 2]]
W = [[1, 0, 3], [1, 2, 1], [0, 1, 1]]
Conv1d Implementation p=2 s=1: [ 5. 14. 16. 26. 24. 34. 19. 22.] <- Correct

conv1d for (German Book, 2. Auflage, Seite 493)
x= [3, 2, 1, 7, 1, 2, 5, 4]
w= [0.5, 0.75, 1.0, 0.25]
Conv1d Implementation p=0 s=2: [7. 9.] <- Wrong
Expected: [7 9 8]

conv2d works for
X= [[1, 3, 2, 4], [5, 6, 1, 3], [1, 2, 0, 2], [3, 4, 3, 2]]
W= [[1, 0, 3], [1, 2, 1], [0, 1, 1]]
Correct Result

conv2d for (German Book, 2. Auflage, Seite 498)
X= [[2, 1, 2], [5, 0, 1], [1, 7, 3]]
W= [[0.5, 0.7, 0.4], [0.3, 0.4, 0.1], [0.5, 1.0, 0.5]]
Conv2d Implementation p=1,1 s=2,2: [[4.6]]
Expected: [[4.6, 1.6], [7.5, 2.9]

Even if both implementations are called naive, they should return correct results.
I think the problem is the end-condition in the for-statement.

conv1d
book: for j in range(0, int(len(x)/s), s):
new: for j in range(0, len(x_padded)-len(w_rot)+1, s):

conv2d
book:
for i in range(0, int((X_padded.shape[0] - W_rot.shape[0]) / s[0]) + 1, s[0]): # Fehler bei s=2,2
for j in range(0, int((X_padded.shape[1] - W_rot.shape[1]) / s[1]) + 1, s[1]): # Fehler bei s=2,2
new:
for i in range(0, int((X_padded.shape[0] - W_rot.shape[0]) ) + 1, s[0]):
for j in range(0, int((X_padded.shape[1] - W_rot.shape[1]) ) + 1, s[1]):

@rasbt
Copy link
Owner

rasbt commented Feb 3, 2021

Thanks for the note. It's been a few years ago, but I vaguely remember this issue. I believe we had fixed that in the 3rd edition: https://github.com/rasbt/python-machine-learning-book-3rd-edition/blob/master/ch15/ch15_part1.ipynb

@JohannWO
Copy link
Author

JohannWO commented Feb 4, 2021

Had a look at the Jupyter Notebook of 3rd Edition
conv1d:
the code was changed to
3rd edition: for i in range(0, int((len(x_padded) - len(w_rot)) / s) + 1, s):
when you use this code with example in Out[5] it returns the wrong result: Conv1d Implementation p=0 s=2: [7. 9.]
i think the "/ s" is wrong, as you take s as step

conv2d:
the code was not changed.
example in Out[11] still results in [[4.6]]

I only compared the for-statements. If there were further changes in the code, i did not check this.

@rasbt
Copy link
Owner

rasbt commented Feb 5, 2021

You are right. Arg, yeah, the /s shouldn't be there. Probably didn't notice due to setting s=1. Need to check this more thoroughly and update it. I will add a reference on the 3rd editions issue tracker so that it can be fixed in the 4th edition.

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

No branches or pull requests

2 participants