Fix empty tensor for unique_dim #19000

codexetreme · 2019-04-07T19:20:03Z

zasdfgbnm

Hi @codexetreme, could you please complete this PR first, i.e. to add unit test, and do the same check for GPU? By adding unit test, you could basically find problem by yourself, and change it without having to ask help. The error messages in circle CI should be fairly helpful. Also, please resolve merge conflicts.

aten/src/ATen/native/Unique.cpp

codexetreme

I have fixed merge conflicts, the other irregularities you mentioned. Also I am having a bit of difficulty writing tests. Where are these supposed to go exactly? there are many test folders. Should I add the tests in test_torch.py or in some other test folder?

aten/src/ATen/native/cuda/Unique.cu

aten/src/ATen/native/Unique.cpp

zasdfgbnm · 2019-04-11T02:21:56Z

@codexetreme Sorry for the late reply, yes, for the test, you need to write it at test_torch.py. Just search for test_unique_dim in that file. I will check your work later. But in the mean time, could you resolve the merge conflicts again?

zasdfgbnm · 2019-04-11T02:23:23Z

BTW, I am working on unique also, so some of my work might frequently cause merge conflicts for this. You might also continue your work, and resolve the conflicts later, which might be of less trouble. It's your choice.

zasdfgbnm · 2019-04-11T02:26:21Z

@codexetreme Also, whenever you see build failures from CI, could you also please check where is error and try fixing it? :)

codexetreme · 2019-04-11T16:31:23Z

Alright, I am looking into the CI logs... I'll write the tests and try to fix the CI bugs.

You might also continue your work, and resolve the conflicts later, which might be of less trouble.

I will do this...

codexetreme · 2019-04-11T19:20:09Z

So in some of the CI logs, the error that is said is :

Reading package lists...
Building dependency tree...
Reading state information...
Some packages could not be installed. This may mean that you have
requested an impossible situation or if you are using the unstable
distribution that some required packages have not yet been created
or been moved out of Incoming.
The following information may help to resolve the situation:

The following packages have unmet dependencies:
nvidia-docker2 : Depends: nvidia-container-runtime (= 2.0.0+docker18.06.2-1) but 2.0.0+docker18.06.2-2 is to be installed
E: Unable to correct problems, you have held broken packages.
Exited with code 100

Does this mean that the containers have not been properly instantiated or that the code in the commit is broken? And what to do for these?

zasdfgbnm · 2019-04-12T04:19:39Z

@codexetreme For these kind of failures, you could do a "@pytorchbot retest this please"

codexetreme · 2019-04-13T06:34:12Z

@pytorchbot retest this please

zasdfgbnm · 2019-04-13T14:16:27Z

@codexetreme The bot is currently broken. You may try again next week.

zasdfgbnm · 2019-04-15T19:12:51Z

@pytorchbot retest this please

codexetreme · 2019-04-15T21:22:24Z

The same issue persists with the tests. What should be done ?

zasdfgbnm · 2019-04-15T22:36:11Z

@codexetreme I think you need to resolve the conflict and merge with the latest master branch now. They must have updated CircleCI configurations.

codexetreme · 2019-04-18T10:35:50Z

Sorry for the late reply (uni assignments take up time :( ). Could you tell me the use of the return_counts bool? my code is failing as I am not returning a third tensor that should contain the counts tensor. According to my understanding, if the tensor is empty should I just return an empty tensor for it? (since there are no elements in the tensor and count would be zero anyway)

zasdfgbnm · 2019-04-18T17:17:23Z

@codexetreme Yes

codexetreme · 2019-04-23T09:15:37Z

In the Unique.cu file, in unique_dim_cuda_template input_flat, is declared twice,and this is giving the error

Apr 23 07:48:27 /var/lib/jenkins/workspace/aten/src/ATen/native/cuda/Unique.cu(168): error: "input_flat" has already been declared in the current scope

a simple fix is to just rename its usages here, since I dont see these particular lines being used elsewhere.

Tensor input_flat = self.transpose(dim, 0);
auto orig_sizes = input_flat.sizes().vec();
input_flat = input_flat.contiguous().view({input_flat.size(0), -1});

codexetreme · 2019-05-04T16:12:39Z

So, I have made the changes you requested, I changed the logic so now it handles the self.size(dim) !=0 case as well. Can you please check and let me know it is right?

zasdfgbnm

It looks much better now. Could you please also fix the linter failures in Travis CI?

./test/test_torch.py:10963:17: E128 continuation line under-indented for visual indent
./test/test_torch.py:10964:17: E128 continuation line under-indented for visual indent
./test/test_torch.py:10965:17: E128 continuation line under-indented for visual indent
./test/test_torch.py:10966:17: E128 continuation line under-indented for visual indent
./test/test_torch.py:10969:17: E128 continuation line under-indented for visual indent
./test/test_torch.py:10970:17: E128 continuation line under-indented for visual indent
./test/test_torch.py:10971:17: E128 continuation line under-indented for visual indent
./test/test_torch.py:10972:17: E128 continuation line under-indented for visual indent
The command "flake8" exited with 1.

aten/src/ATen/native/Unique.cpp

codexetreme · 2019-05-05T08:09:19Z

Ok, so I have fixed the linting, what to do next?

aten/src/ATen/native/Unique.cpp

codexetreme · 2019-05-07T04:30:03Z

Why is that 1 particular test failing? The other cuda based tests seem to have passed...

zasdfgbnm

@codexetreme That failure is not your fault, just ignore it. The code LGTM now. @VitalyFedyunin Could you please take a look and merge?

codexetreme · 2019-05-12T13:26:37Z

When will this be merged? Kinda curious .....

zasdfgbnm · 2019-05-12T20:20:48Z

@pytorchbot rebase this please

zasdfgbnm · 2019-05-12T20:21:27Z

kindly ping @VitalyFedyunin :)

Could you please take a look at this? This does not have API change and should be easy to review.

VitalyFedyunin · 2019-05-13T15:05:03Z

Sure, I was hoping to have #20017 fixed (this helps to write tests quickly), guess I will do workaround.

VitalyFedyunin · 2019-05-22T21:14:31Z

aten/src/ATen/native/Unique.cpp

+    // check how many zero dimentions exist
+    auto num_zero_dims = std::count(sizes.begin(), sizes.end(), 0);
+
+    // tensor is not well formed as it has 0 sized dimentions


nit: dimenSions, here and everywhere

fixed this :)

VitalyFedyunin · 2019-05-22T21:18:20Z

test/test_torch.py

+
+            # test not a well formed tensor
+            # Checking for runtime error, as this is the expected behaviour
+            self.assertRaises(


nit: It is cleaner to write

with self.assertRaises(RuntimeError): torch.unique( x_ill_formed_empty, return_inverse=True, return_counts=True, dim=1)

I have made the change like suggested :)

facebook-github-bot

@VitalyFedyunin has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

VitalyFedyunin · 2019-05-23T15:54:38Z

Could you please cleanup training whitespaces

facebook-github-bot

@VitalyFedyunin has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Summary: Fixes: #18408 cc: zasdfgbnm Pull Request resolved: pytorch/pytorch#19000 Reviewed By: ezyang Differential Revision: D15470136 Pulled By: VitalyFedyunin fbshipit-source-id: daf71566b4dbdc91927d164f813b5ee8645af1a2

facebook-github-bot · 2019-05-29T23:42:53Z

@VitalyFedyunin merged this pull request in 0ffd20c.

codexetreme added 3 commits April 7, 2019 18:08

trying to fix pytorch#18408

20ae623

fixed a small error

033c003

fixed a small error in code for pytorch#18408

fd80b91

zasdfgbnm requested changes Apr 8, 2019

View reviewed changes

aten/src/ATen/native/Unique.cpp Outdated Show resolved Hide resolved

aten/src/ATen/native/Unique.cpp Outdated Show resolved Hide resolved

codexetreme added 3 commits April 9, 2019 11:26

Merge branch 'master' of https://github.com/codexetreme/pytorch

53b19d2

Fixed typos, changed logic for well_formed_tensor

e89e6f8

added code to cuda version as well

6e1de27

codexetreme commented Apr 9, 2019

View reviewed changes

aten/src/ATen/native/cuda/Unique.cu Outdated Show resolved Hide resolved

aten/src/ATen/native/Unique.cpp Outdated Show resolved Hide resolved

aten/src/ATen/native/Unique.cpp Outdated Show resolved Hide resolved

zasdfgbnm self-assigned this Apr 11, 2019

codexetreme added 2 commits April 18, 2019 14:25

merged from upstream

5640966

Added tests to check for _unique_dim does not support empty tensor

f66e2ab

codexetreme added 4 commits April 19, 2019 03:19

added counts return tensor

9fb6c48

Merge branch 'master' of https://github.com/pytorch/pytorch

fc39408

Merge branch 'master' of https://github.com/codexetreme/pytorch

157c06a

Bug Fix and added test for return_counts

085d067

codexetreme force-pushed the master branch from f40f123 to 4123d6d Compare May 4, 2019 15:39

zasdfgbnm reviewed May 4, 2019

View reviewed changes

aten/src/ATen/native/Unique.cpp Outdated Show resolved Hide resolved

aten/src/ATen/native/Unique.cpp Outdated Show resolved Hide resolved

Fixed linting and minor issue

2eea140

zasdfgbnm reviewed May 6, 2019

View reviewed changes

aten/src/ATen/native/Unique.cpp Outdated Show resolved Hide resolved

aten/src/ATen/native/Unique.cpp Outdated Show resolved Hide resolved

aten/src/ATen/native/Unique.cpp Outdated Show resolved Hide resolved

removed redundant condition check and fixed linting

4877df1

zasdfgbnm approved these changes May 7, 2019

View reviewed changes

zasdfgbnm changed the title ~~[WIP] Fix empty tensor for unique_dim~~ Fix empty tensor for unique_dim May 7, 2019

Merge remote-tracking branch 'origin/master' into HEAD

c150085

pytorchbot added module: cuda Related to torch.cuda, and CUDA support in general module: operators labels May 12, 2019

VitalyFedyunin approved these changes May 22, 2019

View reviewed changes

facebook-github-bot reviewed May 23, 2019

View reviewed changes

facebook-github-bot closed this in 0ffd20c May 29, 2019

facebook-github-bot added the merged label May 29, 2019

codexetreme added 3 commits June 22, 2019 07:18

fixed the typos and tests

d6fe5e7

Merge branch 'master' of https://www.github.com/codexetreme/pytorch

73dd22d

removed trailing white spaces

84ce13e

ezyang added the open source label Jun 24, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix empty tensor for unique_dim #19000

Fix empty tensor for unique_dim #19000

codexetreme commented Apr 7, 2019

zasdfgbnm left a comment •

edited

Loading

codexetreme left a comment

zasdfgbnm commented Apr 11, 2019

zasdfgbnm commented Apr 11, 2019

zasdfgbnm commented Apr 11, 2019

codexetreme commented Apr 11, 2019 •

edited

Loading

codexetreme commented Apr 11, 2019

zasdfgbnm commented Apr 12, 2019

codexetreme commented Apr 13, 2019

zasdfgbnm commented Apr 13, 2019

zasdfgbnm commented Apr 15, 2019

codexetreme commented Apr 15, 2019

zasdfgbnm commented Apr 15, 2019

codexetreme commented Apr 18, 2019 •

edited

Loading

zasdfgbnm commented Apr 18, 2019

codexetreme commented Apr 23, 2019 •

edited

Loading

codexetreme commented May 4, 2019

zasdfgbnm left a comment

codexetreme commented May 5, 2019 •

edited

Loading

codexetreme commented May 7, 2019

zasdfgbnm left a comment •

edited

Loading

codexetreme commented May 12, 2019

zasdfgbnm commented May 12, 2019

zasdfgbnm commented May 12, 2019 •

edited

Loading

VitalyFedyunin commented May 13, 2019

VitalyFedyunin May 22, 2019

codexetreme May 23, 2019

VitalyFedyunin May 22, 2019

codexetreme May 23, 2019

facebook-github-bot left a comment

VitalyFedyunin commented May 23, 2019

facebook-github-bot left a comment

facebook-github-bot commented May 29, 2019

Fix empty tensor for unique_dim #19000

Fix empty tensor for unique_dim #19000

Conversation

codexetreme commented Apr 7, 2019

zasdfgbnm left a comment • edited Loading

Choose a reason for hiding this comment

codexetreme left a comment

Choose a reason for hiding this comment

zasdfgbnm commented Apr 11, 2019

zasdfgbnm commented Apr 11, 2019

zasdfgbnm commented Apr 11, 2019

codexetreme commented Apr 11, 2019 • edited Loading

codexetreme commented Apr 11, 2019

zasdfgbnm commented Apr 12, 2019

codexetreme commented Apr 13, 2019

zasdfgbnm commented Apr 13, 2019

zasdfgbnm commented Apr 15, 2019

codexetreme commented Apr 15, 2019

zasdfgbnm commented Apr 15, 2019

codexetreme commented Apr 18, 2019 • edited Loading

zasdfgbnm commented Apr 18, 2019

codexetreme commented Apr 23, 2019 • edited Loading

codexetreme commented May 4, 2019

zasdfgbnm left a comment

Choose a reason for hiding this comment

codexetreme commented May 5, 2019 • edited Loading

codexetreme commented May 7, 2019

zasdfgbnm left a comment • edited Loading

Choose a reason for hiding this comment

codexetreme commented May 12, 2019

zasdfgbnm commented May 12, 2019

zasdfgbnm commented May 12, 2019 • edited Loading

VitalyFedyunin commented May 13, 2019

VitalyFedyunin May 22, 2019

Choose a reason for hiding this comment

codexetreme May 23, 2019

Choose a reason for hiding this comment

VitalyFedyunin May 22, 2019

Choose a reason for hiding this comment

codexetreme May 23, 2019

Choose a reason for hiding this comment

facebook-github-bot left a comment

Choose a reason for hiding this comment

VitalyFedyunin commented May 23, 2019

facebook-github-bot left a comment

Choose a reason for hiding this comment

facebook-github-bot commented May 29, 2019

zasdfgbnm left a comment •

edited

Loading

codexetreme commented Apr 11, 2019 •

edited

Loading

codexetreme commented Apr 18, 2019 •

edited

Loading

codexetreme commented Apr 23, 2019 •

edited

Loading

codexetreme commented May 5, 2019 •

edited

Loading

zasdfgbnm left a comment •

edited

Loading

zasdfgbnm commented May 12, 2019 •

edited

Loading