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.

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

DOC: Fix a typo in description and add an example of numpy.tensordot #23547

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

John-Venti
Copy link

@John-Venti John-Venti commented Apr 7, 2023


Updated comment on 7.2.2023:


  1. I modified the description,

    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
    Nth axis in b last.

    to

    When axes is integer_like, the sequence of axes for evaluation
    will be: from the -Nth axis to the -1th axis in a,
    and from the 0th axis to (N-1)th axis in b.
    For example, axes = 2 is the equal to
    axes = [[-2, -1], [0, 1]].
    When N-1 is smaller than 0, or when -N is larger than -1,
    the element of 'a' and 'b' are defined as the 'axes'.

    This is because the source code of np.tensordot() is

    except Exception:
    axes_a = list(range(-axes, 0))
    axes_b = list(range(0, axes))
    else:
    axes_a, axes_b = axes

    Therefore, (N-1)th is more proper than Nth. for example, when N=10, list(range(0,10)) and list(range(-10,0)) are the following, respectively:

    [0, 1, 2, 3, 4, 5, 6, 7, 8, 9] 
    and
    [-10, -9, -8, -7, -6, -5, -4, -3, -2, -1]

Because tensordot() in other libraries ,like Pytorch, doesn't have the usage of integer_like as axes input. This method is only exist in Numpy, therefore enough instructions are needed. And I think this method is created for the author of numpy.tensordot() hopes that users can write more concise codes.
The results are different for axes = [[-2, -1], [0, 1]] and axes = [[-1, -2], [0, 1]]. Therefore, I think it's important to emphasise that, and I guess the author of this part also hope to express that.

  1. I add an example to enhance the description of integer_like:
     "traditional" examples:
    >>> # An example on integer_like
    >>> a_0 = np.array([[1, 2], [3, 4]])
    >>> b_0 = np.array([[5, 6], [7, 8]])
    >>> c_0 = np.tensordot(a_0, b_0, axes=0)
    >>> c_0.shape
    (2, 2, 2, 2)
    >>> c_0
    array([[[[ 5,  6],
             [ 7,  8]],
            [[10, 12],
             [14, 16]]],
           [[[15, 18],
             [21, 24]],
            [[20, 24],
             [28, 32]]]])


Reference

I think this is an important issue, for many people can't understand the description of the tutorial 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, for integer_like is a specific usage of tensordot() in numpy. Spending time to understand how to use 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.

Thank you for your review and help.

@John-Venti John-Venti changed the title modify the description and example of np-tensordot DOC: modify the description and example of np-tensordot (#174 of numpy-tutorials) Apr 7, 2023
@John-Venti John-Venti changed the title DOC: modify the description and example of np-tensordot (#174 of numpy-tutorials) DOC: Modify the description and example of np.tensordot() (#174 of numpy-tutorials) Apr 7, 2023
@John-Venti
Copy link
Author

John-Venti commented Apr 9, 2023

@charris I fixed the variable name conflict in my last PR. I think this version is stable.
could you check it again?
you can find the problem description at numpy/numpy-tutorials#174 or #23586

@John-Venti John-Venti changed the title DOC: Modify the description and example of np.tensordot() (#174 of numpy-tutorials) DOC: Modify the description and example of np.tensordot() (https://github.com/numpy/numpy-tutorials/issues/174) Apr 9, 2023
@John-Venti John-Venti changed the title DOC: Modify the description and example of np.tensordot() (https://github.com/numpy/numpy-tutorials/issues/174) DOC: Modify the description and example of np.tensordot() (numpy/numpy-tutorials # 174) Apr 9, 2023
@John-Venti
Copy link
Author

John-Venti commented Apr 11, 2023

@charris
I rectified some conflicts I submitted before, for I didn't notice original codes behind the example I added use the same variable name.
I believe this version is stable and correct now.
could you check that again?
I think it is an important modification for the tutorial of np.tensordot(). Because the description of the argument exists some typos which will mislead many new users, and I find many people shows they can't understand the tutorial.

@John-Venti John-Venti changed the title DOC: Modify the description and example of np.tensordot() (numpy/numpy-tutorials # 174) DOC: Modify the description and example of np.tensordot() (https://github.com/numpy/numpy/issues/23586) Apr 14, 2023
@John-Venti John-Venti changed the title DOC: Modify the description and example of np.tensordot() (https://github.com/numpy/numpy/issues/23586) DOC: Modify the description and example of np.tensordot() #23586) Apr 14, 2023
@John-Venti John-Venti changed the title DOC: Modify the description and example of np.tensordot() #23586) DOC: Modify the description and example of np.tensordot() issue#23586) Apr 14, 2023
@John-Venti John-Venti changed the title DOC: Modify the description and example of np.tensordot() issue#23586) DOC: Fix a typo in description and add an example of np.tensordot() issue#23586) Apr 15, 2023
@InessaPawson InessaPawson added the triage review Issue/PR to be discussed at the next triage meeting label May 3, 2023
numpy/core/numeric.py Outdated Show resolved Hide resolved
Comment on lines 974 to 977
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
Nth axis in `b` last.
(N-1)th axis in `b` last. When N-1 is smaller than 0, or when -N is larger
than -1, the element of 'a' and 'b' are defined as the 'axes'.
Copy link
Member

Choose a reason for hiding this comment

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

I think this whole section should be removed. It is less clear than the explanation above (in the Parameters section).

Copy link
Author

Choose a reason for hiding this comment

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

I also think this part is a little redundant.
However, I think this part also has certain significance.

  1. I think it should be claimed what would happen when N is less than 0, because there is no limitation or error warning when N < 0.
  2. the orders of a and b axes are important and it's not clear in the parameters section.

Therefore, I add some details to this part to make it clearer and more meaningful.

Copy link
Member

Choose a reason for hiding this comment

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

Let's see what other reviewers think

Copy link
Author

Choose a reason for hiding this comment

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

about reason 2.
please let me explain more.
the results are different for axes = [[-2, -1], [0, 1]] and axes = [[-1, -2], [0, 1]]. Therefore, I think it's important to emphasise that, and I guess the author of this part also hope to express that.
Because tensordot() in other libraries ,like Pytorch, doesn't have the usage of integer_like as axes input. This method is only exist in Numpy, therefore enough instructions are needed. And I think this method is created for the author of numpy.tensordot() hopes that users can write more concise codes.

@mattip mattip removed the triage review Issue/PR to be discussed at the next triage meeting label Jun 11, 2023
Comment on lines 974 to 980
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
Nth axis in `b` last.
When `axes` is integer_like, the sequence of axes for evaluation
will be: from the -Nth axis to the -1th axis in `a`,
and from the 0th axis to (N-1)th axis in `b`.
For example, ``axes = 2`` is the equal to
``axes = [[-2, -1], [0, 1]]``.
When N-1 is smaller than 0, or when -N is larger than -1,
the element of 'a' and 'b' are defined as the 'axes'.
Copy link
Author

Choose a reason for hiding this comment

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

Agree with @mattip, this part was a little unclear and redundant before.
Therefore, I add some details to help express more clearly.

Comment on lines 974 to 977
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
Nth axis in `b` last.
(N-1)th axis in `b` last. When N-1 is smaller than 0, or when -N is larger
than -1, the element of 'a' and 'b' are defined as the 'axes'.
Copy link
Author

Choose a reason for hiding this comment

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

about reason 2.
please let me explain more.
the results are different for axes = [[-2, -1], [0, 1]] and axes = [[-1, -2], [0, 1]]. Therefore, I think it's important to emphasise that, and I guess the author of this part also hope to express that.
Because tensordot() in other libraries ,like Pytorch, doesn't have the usage of integer_like as axes input. This method is only exist in Numpy, therefore enough instructions are needed. And I think this method is created for the author of numpy.tensordot() hopes that users can write more concise codes.

@John-Venti John-Venti changed the title DOC: Fix a typo in description and add an example of np.tensordot() issue#23586) DOC: Fix a typo in description and add an example of np.tensordot() Jun 30, 2023
@John-Venti John-Venti changed the title DOC: Fix a typo in description and add an example of np.tensordot() DOC: Fix a typo in description and add an example of $numpy.tensordot$ Jun 30, 2023
@John-Venti John-Venti changed the title DOC: Fix a typo in description and add an example of $numpy.tensordot$ DOC: Fix a typo in description and add an example of $$numpy.tensordot$$ Jun 30, 2023
@John-Venti John-Venti changed the title DOC: Fix a typo in description and add an example of $$numpy.tensordot$$ DOC: Fix a typo in description and add an example of numpy.tensordot Jun 30, 2023
@John-Venti John-Venti requested a review from mattip August 13, 2023 17:58
Copy link
Contributor

@linus-md linus-md left a comment

Choose a reason for hiding this comment

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

I have a few minor suggestions.

numpy/core/numeric.py Outdated Show resolved Hide resolved
numpy/core/numeric.py Outdated Show resolved Hide resolved
numpy/core/numeric.py Outdated Show resolved Hide resolved
@John-Venti
Copy link
Author

I have a few minor suggestions.

Thanks for your suggestions! I modified them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Awaiting a code review
Development

Successfully merging this pull request may close these issues.

None yet

4 participants