add functions in nn.utils.rnn to get the last step of a PackedSequence object #1375

WarBean · 2017-04-27T07:58:50Z

No description provided.

…e object

apaszke

Hey, sorry for a late review!

Can you please add some tests for these functions?

torch/nn/utils/rnn.py

+    for i, length in enumerate(lengths):
+        index = sum([min(length - 1, l) for l in lengths]) + i
+        indices.append(index)
+    return indices


torch/nn/utils/rnn.py

+def get_last_step_tensor(packed_sequence, lengths):
+    """Extract the last step of each sequence of a :class:`PackedSequence` object.
+
+    It is useful for rnn's output.


nrbrd · 2017-07-18T09:13:38Z

Hi @WarBean, any update on this? Can I continue your PR?

reiinakano · 2017-08-15T15:00:05Z

Hi @WarBean and @apaszke, don't know how things work in this situation but would it be okay if someone else continued this PR? Can I just create my own based off of this? This is a very useful function I'd like added in Pytorch soon.

apaszke · 2017-08-16T02:56:40Z

I'll accept updated versions of this PR too. Just start a new branch with this commit and put your updates in a new one

WarBean · 2017-08-16T16:08:31Z

Sorry for the delay. I would like to continue the PR but I am not familiar with Github 's PR workflow. Should I close this thread and restart a new one with the amended commit based on the master branch？

apaszke · 2017-08-16T16:12:25Z

Just push a new commit, or force push an amended version of this commit to your branch. GitHub will see this and automatically update this PR with a new version. No need to close anything

reiinakano · 2017-08-17T11:55:28Z

Hi, this is just a suggestion but isn't the lengths parameter unnecessary? The PackedSequence object already stores the lengths. Why not just use that?

Also, you could define get_last_step_tensor as an instance method of PackedSequence. Personally, I think that makes more sense.

What do you think?

WarBean · 2017-08-18T01:52:09Z

@reiinakano Excellent suggestion! Using the self.batches stored inside PackedSequence I can easily use O(n) time to compute the indices.

WarBean · 2017-08-18T02:40:10Z

I don't know how to add tests inside the project but I have write an extra test script:

import torch
from rnn import *
import numpy as np
from torch.autograd import Variable

lengths = np.sort(np.random.randint(1, 100, 128)).tolist()[::-1]
a = Variable(torch.zeros(max(lengths), len(lengths), 13))
for i, length in enumerate(lengths):
    a.data[length - 1, i] = 1
sequence = pack_padded_sequence(a, lengths)
last_step = sequence.last_step_tensor()
print(last_step)
assert torch.sum(last_step.data - 1) == 0

Output:

Variable containing:
    1     1     1  ...      1     1     1
    1     1     1  ...      1     1     1
    1     1     1  ...      1     1     1
       ...          ⋱          ...
    1     1     1  ...      1     1     1
    1     1     1  ...      1     1     1
    1     1     1  ...      1     1     1
[torch.FloatTensor of size 128x13]

Seems nothing wrong.

reiinakano · 2017-08-18T03:19:22Z

@WarBean This is the unit test for pack_padded_sequence https://github.com/pytorch/pytorch/blob/master/test/test_nn.py#L1769. The unit tests are run every time someone makes a change to the codebase.

I believe you should build a unit test for each new function you introduce to make sure their behavior is consistent with every new commit.

WarBean · 2017-08-18T07:06:51Z

Any suggestion on how to pass the checks? Not sure what "no environment variables set" means in Travis CI.

apaszke · 2017-08-19T14:19:44Z

I can't see the error you mentioned. The tests failed because of a bug somewhere in this PR

test/test_nn.py

        expected_data = list(itertools.chain.from_iterable(expected_data))
        expected_data = torch.stack(expected_data, dim=0)
-
+        expected_last_step_data = torch.stack([padded.data[length - 1, i] for i, length in enumerate(lenghts)])


ccarter-cs · 2017-08-30T23:24:02Z

@WarBean You have a transposed character in your test. "lenghts". If you click on the details of the travis build then on one of the builds (there are separate ones for different python versions etc), you will see a log of the test output.

keishinkickback · 2017-12-30T04:58:25Z

@aron-bordin @reiinakano @WarBean @apaszke Has any of you guys added this functionality elsewhere?

apaszke · 2017-12-30T11:29:38Z

I don't think so

* Have Kernel Inherit IrContainer (pytorch#1375) * Kernel<-Fusion Step 1 - Convert ExprSort to StmtSort (pytorch#1376) * Kernel<-Fusion Step 2 - Mutator refactor (pytorch#1377) * Kernel<-Fusion Step 3 - Debug print for expr_eval and type promotion fix (pytorch#1379) * Kernel<-Fusion Step 4 - Have kernel inherit Fusion (pytorch#1380) * Kernel<-Fusion Step 5 - Move lowering passes into their own files (pytorch#1382) * Kernel<-Fusion Step 6 - Remove kir::IrBuilder (pytorch#1383) * Kernel<-Fusion Step 7 - Remove kir functions from ComputeAtMap (pytorch#1384) * Kernel<-Fusion Step 8 - Clean up [lower/executor] utils (pytorch#1387) * Kernel<-Fusion Step 9 - Remove TensorView::fuserTv (pytorch#1388) * Kernel<-Fusion Step 10 - Remove lowerVal/lowerExpr (pytorch#1389) * Kernel<-Fusion Step 11 - Finish cleaning up kir (pytorch#1390)

github-actions · 2022-03-01T17:08:38Z

Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as Stale.
Feel free to remove the Stale label if you feel this was a mistake.
If you are unable to remove the Stale label please contact a maintainer in order to do so.
Stale pull requests will automatically be closed 30 days after being marked Stale

facebook-github-bot · 2022-03-29T18:59:10Z

Hi @WarBean!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

github-actions · 2022-05-28T19:34:05Z

Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as Stale.
Feel free to remove the Stale label if you feel this was a mistake.
If you are unable to remove the Stale label please contact a maintainer in order to do so.
If you want the bot to never mark this PR stale again, add the no-stale label.
Stale pull requests will automatically be closed after 30 days of inactivity.

================================================= Temporarily skip test_conv3d_64bit_indexing - Rocblas API support is requested - SWDEV-383635 & sub task - SWDEV-390218 Skip ddp apply_optim_in_bwd tests for gloo (pytorch#1302) To resolve https://ontrack-internal.amd.com/browse/SWDEV-403530 and https://ontrack-internal.amd.com/browse/SWDEV-419837. For more context check upstream issue pytorch#111834 Add skipIfRocmArch decorator for Navi skips (pytorch#1356) Converted NAVI check as a function (pytorch#1364) * Moved NAVI check to the test file * Revised NAVI check as a function [Navi] [Inductor] Unskip Navi inductor UTs (pytorch#1514) Relates to https://ontrack-internal.amd.com/browse/SWDEV-461590 Bad import in test_torchinductor and skip torchvision related UT (pytorch#1374) skip test_inductor_freezing failing UTs (pytorch#1375) Skip test_mm_triton_kernel_benchmark (pytorch#1376) * Running triton kernel on ROCM only has one GB/s metric reported * Update test_kernel_benchmark.py skip vmapvjpvjp_linalg_householder_product_cuda_float32 (pytorch#1420) skipIfRocm needs msg parameter [NO CP] Updated changes to skip few UTs Imported skipIfRocm in certain test suites (pytorch#1577) Fixes SWDEV-472397 Added functions imports (pytorch#1521) Fixes inductor.test_torchinductor_dynamic_shapes::TestInductorDynamicCUDA::test_item_unbacked_stride_nobreak_cuda Enable test_public_api_surface (pytorch#1601) Fixes SWDEV-462410. Enable this unit test since PyTorch issue pytorch#104012 has been closed. This unit test runs fine on MI100/MI300 and upstream. (cherry picked from commit 0001d4ab5070635cfecc146ee299bbb9fa45ca67) [rocm6.3_internal_testing] Fixed error string assertion in test_invalid_devices (pytorch#1607) Fixes pytorch#8974 (cherry picked from commit a688e0a)

================================================= Temporarily skip test_conv3d_64bit_indexing - Rocblas API support is requested - SWDEV-383635 & sub task - SWDEV-390218 Skip ddp apply_optim_in_bwd tests for gloo (pytorch#1302) To resolve https://ontrack-internal.amd.com/browse/SWDEV-403530 and https://ontrack-internal.amd.com/browse/SWDEV-419837. For more context check upstream issue pytorch#111834 Add skipIfRocmArch decorator for Navi skips (pytorch#1356) Converted NAVI check as a function (pytorch#1364) * Moved NAVI check to the test file * Revised NAVI check as a function [Navi] [Inductor] Unskip Navi inductor UTs (pytorch#1514) Relates to https://ontrack-internal.amd.com/browse/SWDEV-461590 Bad import in test_torchinductor and skip torchvision related UT (pytorch#1374) skip test_inductor_freezing failing UTs (pytorch#1375) Skip test_mm_triton_kernel_benchmark (pytorch#1376) * Running triton kernel on ROCM only has one GB/s metric reported * Update test_kernel_benchmark.py skip vmapvjpvjp_linalg_householder_product_cuda_float32 (pytorch#1420) skipIfRocm needs msg parameter [NO CP] Updated changes to skip few UTs Imported skipIfRocm in certain test suites (pytorch#1577) Fixes SWDEV-472397 Added functions imports (pytorch#1521) Fixes inductor.test_torchinductor_dynamic_shapes::TestInductorDynamicCUDA::test_item_unbacked_stride_nobreak_cuda Enable test_public_api_surface (pytorch#1601) Fixes SWDEV-462410. Enable this unit test since PyTorch issue pytorch#104012 has been closed. This unit test runs fine on MI100/MI300 and upstream. (cherry picked from commit 0001d4ab5070635cfecc146ee299bbb9fa45ca67) [rocm6.3_internal_testing] Fixed error string assertion in test_invalid_devices (pytorch#1607) Fixes pytorch#8974 (cherry picked from commit a688e0a) (cherry picked from commit b966e44) [rocm6.4_internal_testing] Skip non_standard_bool_values tests (pytorch#1880) Fixes SWDEV-509757 (cherry picked from commit 80b4c41) [rocm6.4_internal_testing] [NAVI32] Skipped sdpa_2 test in test_aot_inductor for Navi32 (pytorch#1882) The test fails with assertion error "Tensors are not close" After testing I can confirm that this issue is caused by eager mode execution specific to navi32 during the test_sdpa_2 run. Made a cross reference between navi31, navi32 and mi300. AOTInductor results are all the exact same for all of the archs, only the eager mode fails here for navi32 with 1.5% difference in tensor values from the gpu run. I assume that this happens due to fp16-32-16 conversions in eager mode or missing some if-statements for navi32 specifically. Simple reproducer to check the values for cpu/gpu/eager/aoti runs. [gfx1101_test_sdpa_2_issue_reproducer.txt](https://github.com/user-attachments/files/18676367/gfx1101_test_sdpa_2_issue_reproducer.txt) (cherry picked from commit 896c789) Fixed rocm skip import issue (pytorch#1949) skip_if_rocm does not exist in torch/testing/_internal/common_distributed.py. Use skipIfRocm from torch/testing/_internal/common_utils.py instead. (cherry picked from commit cfb673e) Skip certain unit tests on NAVI (pytorch#1950) This PR is to skip certain unit tests on NAVI only. Fixes SWDEV-509011 - test_sac_ilp.py::TestSACILP::test_sac_ilp_case1 Fixes SWDEV-509311 - test_max_autotune.py::TestMaxAutotune::test_non_contiguous_input_addmm Fixes SWDEV-510738 test_fsdp_sharded_grad_scaler.py::TestShardedGradScalerParityWithDDP::test_sharded_grad_scaler_found_inf (cherry picked from commit e86291a)

add functions in nn.utils.rnn to get the last step of a PackedSequenc…

9a5371d

…e object

apaszke reviewed May 7, 2017

View reviewed changes

WarBean added 2 commits August 18, 2017 10:29

add last_step_indices, last_step_tensor to PackedSequence

ff1b93d

fix bug for the last batch in PackedSequence.last_step_indices

eef98a7

WarBean added 2 commits August 18, 2017 14:27

add unit test for PackedSequence.last_step_tensor

f48e838

Merge branch 'master' into master

b7973a1

ccarter-cs reviewed Aug 30, 2017

View reviewed changes

WarBean added 3 commits September 5, 2017 16:09

fix typo in test/test_nn.py

c056806

merge newest version

7b7717b

fix conflict in test/test_nn.py

8b22c42

keishinkickback mentioned this pull request Jan 9, 2018

duplicate: add functions in nn.utils.rnn to get the last step of a PackedSequence object from #1375 #4553

Closed

ezyang added the open source label Jun 5, 2019

github-actions bot added the Stale label Mar 1, 2022

github-actions bot removed the Stale label Mar 23, 2022

github-actions bot added the Stale label May 28, 2022

github-actions bot closed this Jun 27, 2022

petrex pushed a commit to petrex/pytorch that referenced this pull request Sep 23, 2024

skip test_inductor_freezing failing UTs (pytorch#1375)

526fa61

add functions in nn.utils.rnn to get the last step of a PackedSequence object #1375

add functions in nn.utils.rnn to get the last step of a PackedSequence object #1375

Uh oh!

Conversation

WarBean commented Apr 27, 2017

Uh oh!

apaszke left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

nrbrd commented Jul 18, 2017

Uh oh!

reiinakano commented Aug 15, 2017

Uh oh!

apaszke commented Aug 16, 2017

Uh oh!

WarBean commented Aug 16, 2017

Uh oh!

apaszke commented Aug 16, 2017

Uh oh!

reiinakano commented Aug 17, 2017

Uh oh!

WarBean commented Aug 18, 2017

Uh oh!

WarBean commented Aug 18, 2017

Uh oh!

reiinakano commented Aug 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

WarBean commented Aug 18, 2017

Uh oh!

apaszke commented Aug 19, 2017

Uh oh!

This comment was marked as off-topic.

Uh oh!

ccarter-cs commented Aug 30, 2017

Uh oh!

keishinkickback commented Dec 30, 2017

Uh oh!

apaszke commented Dec 30, 2017

Uh oh!

github-actions bot commented Mar 1, 2022

Uh oh!

facebook-github-bot commented Mar 29, 2022

Action Required

Process

Uh oh!

github-actions bot commented May 28, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

reiinakano commented Aug 18, 2017 •

edited

Loading