Skip to content

DOC: Fix incorrect formula in gradient docstring. #10547

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

Merged
merged 1 commit into from
Mar 1, 2018

Conversation

hobler
Copy link
Contributor

@hobler hobler commented Feb 8, 2018

DOC: Bug fix in explanatory note for the gradient function. Fixes #10545

Original rendered docs here

@charris charris changed the title Update function_base.py DOC: Fix incorrect formula in gradient docstring. Feb 8, 2018
Copy link
Member

@eric-wieser eric-wieser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks correct to me. Maybe we should add another step in the working, doing the second-order tailor expansion, to make that obvious?

@ahaldane
Copy link
Member

I think it's fine to leave out the math. The fix looks right to me too.

But while we're here, can we discuss more fixups nearby? We can add them as github edits once we're decided.

I think we should reword

    Assuming that :math:`f\\in C^{3}` (i.e., :math:`f` has at least 3 continuous
    derivatives) and let be :math:`h_{*}` a non homogeneous stepsize, the
    spacing the finite difference coefficients are computed by minimising
    the consistency error :math:`\\eta_{i}`:

to

    Assuming that :math:`f\\in C^{3}` (i.e., :math:`f` has at least 3 continuous
    derivatives) and let be :math:`h_{*}` a non homogeneous stepsize, we
    minimize the "consistency error" :math:`\\eta_{i}` between the true gradient 
    and an estimate of it as a linear combination of the neighboring grid-points:

Also, just a comment: I was not able to find this precise equation in the 3 references listed, so it took me a minute to understand the assumptions here. There are a ton of finite difference schemes so it would be nice to have clearer link to a reference, to learn more about this one. It looks like we already discussed this here: #8446 (comment)

@eric-wieser
Copy link
Member

@ahaldane: your proposed change looks good to me.

@ahaldane
Copy link
Member

Unless @hobler wants to make that change too, I think we can merge this PR and then make those changes in another PR.

So I'll merge in a few hours if there are no more comments.

@charris charris merged commit a8b8830 into numpy:master Mar 1, 2018
@charris
Copy link
Member

charris commented Mar 1, 2018

Thanks @hobler

@ahaldane Please make a PR with your suggested wording.

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

Successfully merging this pull request may close these issues.

Mistake in documentation of numpy.gradient
4 participants