Fix finite-difference example in conv-layer to use horizontal neighbors#2702
Open
Chessing234 wants to merge 1 commit into
Open
Fix finite-difference example in conv-layer to use horizontal neighbors#2702Chessing234 wants to merge 1 commit into
Chessing234 wants to merge 1 commit into
Conversation
Closes d2l-ai#2669. Section on the [1, -1] edge-detection kernel claims it computes the difference between 'horizontally adjacent pixels' and approximates the first derivative in the 'horizontal direction', but the formulas used the vertical neighbor x_{i+1,j} and the derivative along the i axis: x_{i,j} - x_{(i+1),j} -\partial_i f(i,j) = lim (f(i,j) - f(i+eps,j)) / eps With standard (row i, column j) indexing, moving in j is horizontal and moving in i is vertical. The kernel K = [[1, -1]] is applied along the width (j) axis, so at (i,j) it actually computes x_{i,j} - x_{i,j+1}, and the corresponding derivative is along j, not i. The surrounding prose is already consistent with the horizontal interpretation; this change brings the two formulas on the same line into agreement with it.
Member
|
Hi @Chessing234 - I'm in the middle of refactoring d2l to move it to a more modern tooling (Quarto, latest versions of JAX, TF/Keras and PyTorch, better references, fix performance bugs, etc.). I'll have a first cut of this shortly. Reach out to me by e-mail and we can discuss more (no idea who you are, based off your GH info). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2669.
Bug
The paragraph introducing the
[1, -1]edge-detection kernel inchapter_convolutional-neural-networks/conv-layer.mdsays the kernelbut the two formulas on that same line describe a vertical difference:
Root cause
With the book's (row
i, columnj) indexing, moving injis horizontal; moving iniis vertical. The kernelK = [[1.0, -1.0]]is 1×2 and applied along the width axis, so at position(i, j)cross-correlation producesi.e. a difference between horizontal neighbors, approximating the partial derivative along
j. The prose is correct; the two symbolic expressions used the wrong axis.Why the fix is correct
Three edits on that single line, all swapping the
i-axis offsets forj-axis ones so the formulas match the surrounding prose and the actual behaviour of the 1×2 kernel:x_{i,j} - x_{(i+1),j}→x_{i,j} - x_{i,j+1}-\partial_i f(i,j)→-\partial_j f(i,j)f(i+\epsilon, j)→f(i, j+\epsilon)No other changes; the Python block directly below (
K = d2l.tensor([[1.0, -1.0]])and the worked example on a 6×8 input) is unaffected and already illustrates the horizontal interpretation.