Skip to content

Conversation

@roshinit-a
Copy link

This PR fixes issue #9.

In the notebook, f(x, y) is defined first, but later f is redefined to take
a single vector v containing [x, y]. The finite-difference gradient section
still uses the scalar form f(x, y), causing a TypeError ("f() takes 1 positional
argument but 2 were given").

This fix updates the finite-difference code to call f(v) correctly by
constructing vector inputs. The numerical gradients now match the autograd
results.

@ageron
Copy link
Owner

ageron commented Nov 20, 2025

Hi @roshinit-a,
Oh great catch, and thanks for PR! However, may I ask you to only include the fix itself in this PR? Many cells are also changed, which increases the size of the repo and makes PRs harder to verify. To do that, you can revert to the original version and only edit and run the relevant cells. I'm happy to do it if you prefer.

@roshinit-a
Copy link
Author

Thanks for the feedback! I've updated the PR. I reverted the notebook to the original state to clear all metadata and outputs, and then applied only the specific fix for the finite-difference calculation. It should be much cleaner now.

@ageron
Copy link
Owner

ageron commented Nov 21, 2025

Hi @roshinit-a ,

Ah sorry, we need the outputs unchanged, because some people read the notebooks without running them.
I only meant to revert to the notebook version in the repo like this:

git checkout 10_neural_nets_with_pytorch.ipynb

Then you can open the notebook in Jupyter and only run the cells that you modified (if you need some imports or function definitions, you can just run those: I usually copy the cells I need, run them, then delete them, so the result is as clean as possible).

Things would be much simpler if I didn't include the outputs in the notebooks, but I think it's useful to have them, so it's worth the extra effort. But of course, I'm happy to do this if you prefer.

@roshinit-a
Copy link
Author

Hi @ageron,

Thanks for the guidance! I have updated the PR following your instructions.

I reverted the notebook to the original version to preserve all existing outputs. Then, I used a temporary cell to load the necessary dependencies (torch and the function f) so I could run only the modified cell without executing the rest of the notebook.

The PR now contains just the fix for the finite-difference calculation with its new output, while keeping the rest of the notebook exactly as it was. Let me know if this looks good!

@fbourgey
Copy link

@roshinit-a, going to the Files changed tab, it seems there are still some unwanted meta changes ("execution_count", etc)

@roshinit-a roshinit-a closed this Jan 11, 2026
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.

3 participants