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.

Sign up for GitHub

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

Jump to bottom

[DOC] Issue on page /grid_ufuncs.html #504

Open
dhruvbalwada opened this issue May 24, 2022 · 2 comments · May be fixed by #562
Open

[DOC] Issue on page /grid_ufuncs.html #504

dhruvbalwada opened this issue May 24, 2022 · 2 comments · May be fixed by #562
Labels
docs

Comments

@dhruvbalwada
Copy link
Contributor

dhruvbalwada commented May 24, 2022 •
edited

In cell [2], where diff_forward is defined. It seems like it has been defined in the opposite sense of the usual. (https://mathworld.wolfram.com/ForwardDifference.html)
The usual definition would be diff_a = a[i+1] - a[i], whereas (if I understand roll correctly) it seems that here the function is calculating diff_a = a[i] - a[i+1]. (so there is a sign error).

Also, regardless of the sign error the diff_a would be a variable that ends up on ax1:right, but instead in the paragraph below it seems to suggest that things end up on ax1:left (which is what would happen if we were doing a backward difference.
@jbusecke
Copy link
Contributor

I think that you are right about what the code does. I personally always get confused by the sign of roll. Reading the text it does indeed seem like a backwards difference was intended, so maybe lets just rename the function to backwards_difference and do np.roll(a,1, ...)?
@TomNicholas do you agree?

@dhruvbalwada
Copy link
Contributor Author

Yes, I think if you call it diff_backward, then the rest of the text seems to work out.

@TomNicholas TomNicholas linked a pull request Nov 29, 2022 that will close this issue
Open
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
docs
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix confusion between forwards and backwards diff in grid ufunc docs TomNicholas/xgcm
2 participants
@jbusecke @dhruvbalwada