Skip to content

Conversation

lanluo-nvidia
Copy link
Collaborator

@lanluo-nvidia lanluo-nvidia commented Jun 3, 2024

Description

Numpy has removed itemset api since 2.0, so need to replace the itemset call

Fixes # (issue)

      offsets.itemset(-1, len_embed)

E AttributeError: itemset was removed from the ndarray class in NumPy 2.0. Use arr[index] = value instead.

Collecting numpy (from torchvision==0.18.0->-r /__w/TensorRT/TensorRT/pytorch/tensorrt/tests/py/requirements.txt (line 8))
398
Downloading numpy-2.0.0rc2-cp39-cp39-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (19.3 MB)

Type of change

Please delete options that are not relevant and/or add your own.

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code follows the style guidelines of this project (You can use the linters)
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas and hacks
  • I have made corresponding changes to the documentation
  • I have added tests to verify my fix or my feature
  • New and existing unit tests pass locally with my changes
  • I have added the relevant labels to my PR in so that relevant reviewers are notified

@github-actions github-actions bot added component: conversion Issues re: Conversion stage component: converters Issues re: Specific op converters component: api [Python] Issues re: Python API component: dynamo Issues relating to the `torch.compile` or `torch._dynamo.export` paths labels Jun 3, 2024
@github-actions github-actions bot requested a review from zewenli98 June 3, 2024 14:05
@lanluo-nvidia lanluo-nvidia self-assigned this Jun 3, 2024
@lanluo-nvidia lanluo-nvidia requested a review from narendasan June 3, 2024 14:05
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

There are some changes that do not conform to Python style guidelines:

--- /home/runner/work/TensorRT/TensorRT/py/torch_tensorrt/dynamo/conversion/impl/embedding.py	2024-06-03 14:05:15.070622+00:00
+++ /home/runner/work/TensorRT/TensorRT/py/torch_tensorrt/dynamo/conversion/impl/embedding.py	2024-06-03 14:07:09.016476+00:00
@@ -92,11 +92,11 @@
    if include_last_offset:
        # modify the last index of offsets to the end index
        # however, pytorch doc says if `include_last_offset` is True, the size of offsets
        # is equal to the number of bags + 1. The last element is the size of the input,
        # or the ending index position of the last bag (sequence).
-        offsets_shape=offsets.shape
+        offsets_shape = offsets.shape
        offsets = offsets.flatten()
        offsets[-1] = len_embed
        offsets.reshape(offsets_shape)
    else:
        # add the end index to offsets

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

There are some changes that do not conform to Python style guidelines:

--- /home/runner/work/TensorRT/TensorRT/py/torch_tensorrt/dynamo/conversion/impl/embedding.py	2024-06-03 14:06:12.995886+00:00
+++ /home/runner/work/TensorRT/TensorRT/py/torch_tensorrt/dynamo/conversion/impl/embedding.py	2024-06-03 14:08:07.030401+00:00
@@ -92,11 +92,11 @@
    if include_last_offset:
        # modify the last index of offsets to the end index
        # however, pytorch doc says if `include_last_offset` is True, the size of offsets
        # is equal to the number of bags + 1. The last element is the size of the input,
        # or the ending index position of the last bag (sequence).
-        offsets_shape=offsets.shape
+        offsets_shape = offsets.shape
        offsets = offsets.flatten()
        offsets[-1] = len_embed
        offsets.reshape(offsets_shape)
    else:
        # add the end index to offsets

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

There are some changes that do not conform to Python style guidelines:

--- /home/runner/work/TensorRT/TensorRT/py/torch_tensorrt/dynamo/conversion/impl/embedding.py	2024-06-03 20:24:01.501127+00:00
+++ /home/runner/work/TensorRT/TensorRT/py/torch_tensorrt/dynamo/conversion/impl/embedding.py	2024-06-03 20:26:07.774711+00:00
@@ -92,16 +92,16 @@
    if include_last_offset:
        # modify the last index of offsets to the end index
        # however, pytorch doc says if `include_last_offset` is True, the size of offsets
        # is equal to the number of bags + 1. The last element is the size of the input,
        # or the ending index position of the last bag (sequence).
-        
+
        # Notes: here offsets should always be 1d array
        if len(offsets.shape) != 1:
            raise TypeError(
-            f"The offsets should be in 1d array, here offset shape is {offsets.shape}."
-        )
+                f"The offsets should be in 1d array, here offset shape is {offsets.shape}."
+            )
        offsets[-1] = len_embed
    else:
        # add the end index to offsets
        offsets = np.append(offsets, len_embed)

Copy link
Collaborator

@zewenli98 zewenli98 left a comment

Choose a reason for hiding this comment

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

The modification LGTM. Pending on CI

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

There are some changes that do not conform to Python style guidelines:

--- /home/runner/work/TensorRT/TensorRT/py/torch_tensorrt/dynamo/conversion/impl/embedding.py	2024-06-03 20:28:25.162585+00:00
+++ /home/runner/work/TensorRT/TensorRT/py/torch_tensorrt/dynamo/conversion/impl/embedding.py	2024-06-03 20:30:20.926499+00:00
@@ -96,12 +96,12 @@
        # or the ending index position of the last bag (sequence).

        # Notes: here offsets should always be 1d array
        if len(offsets.shape) != 1:
            raise TypeError(
-            f"The offsets should be in 1d array, here offset shape is {offsets.shape}."
-        )
+                f"The offsets should be in 1d array, here offset shape is {offsets.shape}."
+            )
        offsets[-1] = len_embed
    else:
        # add the end index to offsets
        offsets = np.append(offsets, len_embed)

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

There are some changes that do not conform to Python style guidelines:

--- /home/runner/work/TensorRT/TensorRT/py/torch_tensorrt/dynamo/conversion/impl/embedding.py	2024-06-03 20:39:56.624958+00:00
+++ /home/runner/work/TensorRT/TensorRT/py/torch_tensorrt/dynamo/conversion/impl/embedding.py	2024-06-03 20:41:50.763279+00:00
@@ -97,11 +97,11 @@

        # Notes: here offsets should always be 1d array
        if len(offsets.shape) != 1:
            raise TypeError(
                f"The offsets should be in 1d array, here offset shape is {offsets.shape}."
-        )
+            )
        offsets[-1] = len_embed
    else:
        # add the end index to offsets
        offsets = np.append(offsets, len_embed)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla signed component: api [Python] Issues re: Python API component: conversion Issues re: Conversion stage component: converters Issues re: Specific op converters component: dynamo Issues relating to the `torch.compile` or `torch._dynamo.export` paths
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants