numpy/numpy-tutorials

Typo: maybe a mistake in the document of tensordot

John-Wendell opened this issue · 12 comments

The description of np.tensordot() is
1
however the source code of it is
2
so I think it should be first 0 and N-1th axis in b last rather than "Nth axis in b last".
for example, list(range(0,10)) and list(range(-10,0)) are the following, respectively:
[0, 1, 2, 3, 4, 5, 6, 7, 8, 9]
[-10, -9, -8, -7, -6, -5, -4, -3, -2, -1]

This problem has been perplexing me for a long time.

@Almark-wendy - yes, I think this is a typo in the docs, at least when one think of N as the name of the axis, rather than counting them. Either case, I think your version would be more consistent:

When `axes` is integer_like, the sequence for evaluation will be: first
    the -Nth axis in `a` and 0th axis in `b`, and the -1th axis in `a` and
    (N-1)th axis in `b` last.

Are you interested in opening a documentation fix for it over the https://github.com/numpy/numpy/ repository?

@Almark-wendy - yes, I think this is a typo in the docs, at least when one think of N as the name of the axis, rather than counting them. Either case, I think your version would be more consistent:

When `axes` is integer_like, the sequence for evaluation will be: first
    the -Nth axis in `a` and 0th axis in `b`, and the -1th axis in `a` and
    (N-1)th axis in `b` last.

Are you interested in opening a documentation fix for it over the https://github.com/numpy/numpy/ repository?

Sure, I would be more than happy to help modify this description. I hope more people could learn to use numpy more easily!

@bsipocz I submitted my 'pull request' to the https://github.com/numpy/numpy/ repository.
I modified the description of tensordot and added an example, which has been verified locally.

And I didn't find a file of https://numpy.org/doc/stable/reference/generated/numpy.tensordot.html
could you tell me where to modify the content of this website? I think the content of this website is more important than the comment of source code, for the first thing a large amount of learners look at is the tutorials on the web.

Thanks for the PR.

The API reference docs is auto-generated from the docstrings and has nothing to do with the tutorials themselves (the tutorials are hosted in this repo, but the API docs or the generic numpy website are not).

The updated rendered docs is then accessible through the CI checks under your PR (atm built with circleCI):

https://output.circle-artifacts.com/output/job/a4b15a0a-8fa2-4659-92f3-047c9785c692/artifacts/0/doc/build/html/reference/generated/numpy.tensordot.html

I got it! Thanks!

@bsipocz Hi, bsipocz!
The example of last version I submit ignored the fact that the code that follows used the variables from above.
and I just tested my example without the code behind, which leaded a mistake.
Please forgive me for being involved in open source for the first time.
I rectified it and submitted a new version PR numpy/numpy#23547
I am sure there is no problem this time.
If you have time, please help me review it. Thanks!

@bsipocz I find my rectified PR numpy/numpy#23547 has not been noticed for 5 days. If you have time, can you help me check that? or remind the staff of numpy to check and merge that?
I think it is an important change, because I find many people show that they can't understand the tutorial in stack overflow or other places, for many learners won't to check the source code.

@bsipocz I find this PR numpy/numpy#23547 has not been noticed for 2 weeks.
I also added a issue to describe the typo in detail at numpy/numpy numpy/numpy#23586
If there are some problems in my PR, please tell me, for I'm a hardware engineer and not familiar with github.
I hope numpy can be better and better with the joint efforts of everyone.

@Jianyu-Wen this is not uncommon for open-source projects - review of PRs is largely done by volunteers in their free-time. The trick here is convincing folks that the proposed change is an improvement, and the lack of response likely means that the case has not yet been sufficiently made. I personally don't have expertise pertaining to tensordot, so I can't judge whether the proposal is correct/an improvement.

@Jianyu-Wen this is not uncommon for open-source projects - review of PRs is largely done by volunteers in their free-time. The trick here is convincing folks that the proposed change is an improvement, and the lack of response likely means that the case has not yet been sufficiently made. I personally don't have expertise pertaining to tensordot, so I can't judge whether the proposal is correct/an improvement.

Thanks for your explanation!
But I think this is not the problem of the theory of tensordot(). The problem is the source code is conflicted with the description. The details are in numpy/numpy#23586
And I think this is an important issue, for many people can't understand the descriprion of the tutoria;l of np.tensordot(), especially the using of the integer_like parameters,
for example:

  1. https://medium.com/analytics-vidhya/tensordot-explained-6673cfa5697f shows that
    1
  2. https://stackoverflow.com/questions/51989572/how-does-numpy-tensordot-function-works-step-by-step shows that
    2
  3. https://stackoverflow.com/questions/66214690/numpy-tensordot-axes-2 shows that
    3
  4. ....

many people have to do many experiments to understand what happens, it does take much time and kill people's interests to use it. I also take 2 days to understand it and find the conflict. For efficiency reasons, many people prefer to replace this function with some other functions, which I think will make the function lose it's meaning.
Therefore, I hope to fix the problem as soon as possible.

What's more, I think it is better to add some illustrations about the definition of axes, or the link of it.
Because I also found many people misunderstand the definition of it on the internet.
And I'm happy to provide the illustration, if you think this is reasonable.
I hope people can enjoy using numpy in their work.

I'm going to close this one here - future discussion should happen on the #23547. Thanks @John-Venti !