update s3 test cases #464

ydaiming · 2022-05-25T16:41:41Z

Please read through our contribution guide prior to
creating your pull request.

Note that there is a section on requirements related to adding a new DataPipe.

Fixes #460

Changes

update the s3 test cases due to an update in the public dataset

ydaiming · 2022-05-25T16:44:17Z

@ejguan Could you help enable the datapipes test?

ejguan · 2022-05-25T16:52:47Z

@ydaiming Sorry what do you mean?

ydaiming · 2022-05-25T16:54:04Z

@ejguan the datapipes tests weren't run automatically because there's no changes made in datapipes source code. Just added a blank line to enable the datapipes test.

ejguan · 2022-05-25T16:58:17Z

@ejguan the datapipes tests weren't run automatically because there's no changes made in datapipes source code. Just added a blank line to enable the datapipes test.

Weird. We don't have such advanced mechanism using GHA. Anyways, as long as the tests start working, it's fine.

ejguan · 2022-05-25T17:00:32Z

It seems GHA stuck in queued again. We have to wait for a while...

ydaiming · 2022-05-25T17:06:44Z

How long a while are we expecting? ...

ejguan · 2022-05-25T17:12:54Z

How long a while are we expecting? ...

Emmm. Can't estimate the timeline, this depends on Github. Could you verify this is going to work on your local env?

ydaiming · 2022-05-25T17:43:17Z

I've tested on my remote ubuntu EC2 instance and local MacOS system. Both build and pass the test without the ValueError.

ejguan · 2022-05-25T17:50:26Z

I've tested on my remote ubuntu EC2 instance and local MacOS system. Both build and pass the test without the ValueError.

Interesting. I applied your patch in my env on AWS cluster but I still got the same Error.
The Error comes from iterating the s3_lister_dp not from comparing result with input[1].

data/test/test_remote_io.py

Line 239 in ec83d11

s3_lister_dp = S3FileLister(IterableWrapper(input[0]), region="us-west-2")

ydaiming · 2022-05-25T18:05:03Z

Could you provide a longer backtrace? I'm not able to see that error on both instances.

EDIT: tested on another ubuntu EC2 instance, and still no errors.

ydaiming · 2022-05-25T18:34:48Z

@ejguan the datapipes tests are passing in this PR, I'm bit stuck on reproducing the errors you see. Is there any other way that you think we can continue the debug and fix?

ejguan · 2022-05-25T18:52:00Z

@ejguan the datapipes tests are passing in this PR, I'm bit stuck on reproducing the errors you see. Is there any other way that you think we can continue the debug and fix?

Here is the Error I encountered using your branch:

test_s3_io_iterdatapipe (__main__.TestDataPipeRemoteIO) ... ERROR

======================================================================
ERROR: test_s3_io_iterdatapipe (__main__.TestDataPipeRemoteIO)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/shared/erjia/data/test/test_remote_io.py", line 237, in test_s3_io_iterdatapipe
    self.assertEqual(sum(1 for _ in s3_lister_dp), input[1], f"{input[0]} failed")
  File "/shared/erjia/data/test/test_remote_io.py", line 237, in <genexpr>
    self.assertEqual(sum(1 for _ in s3_lister_dp), input[1], f"{input[0]} failed")
  File "/shared/erjia/pytorch/torch/utils/data/datapipes/_typing.py", line 501, in wrap_generator
    response = gen.send(None)
  File "/shared/erjia/data/torchdata/datapipes/iter/load/s3io.py", line 64, in __iter__
    urls = self.handler.list_files(prefix)
ValueError: Access Denied
This exception is thrown by __iter__ of S3FileListerIterDataPipe(length=-1, source_datapipe=IterableWrapperIterDataPipe)

----------------------------------------------------------------------
Ran 1 test in 0.502s

FAILED (errors=1)
srun: error: a100-st-p4d24xlarge-1: task 0: Exited with exit code 1

Let me create a script and run gdb for you.

ydaiming · 2022-05-25T18:55:40Z

@ejguan the datapipes tests are passing in this PR, I'm bit stuck on reproducing the errors you see. Is there any other way that you think we can continue the debug and fix?

Here is the Error I encountered using your branch:

test_s3_io_iterdatapipe (__main__.TestDataPipeRemoteIO) ... ERROR

======================================================================
ERROR: test_s3_io_iterdatapipe (__main__.TestDataPipeRemoteIO)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/shared/erjia/data/test/test_remote_io.py", line 237, in test_s3_io_iterdatapipe
    self.assertEqual(sum(1 for _ in s3_lister_dp), input[1], f"{input[0]} failed")
  File "/shared/erjia/data/test/test_remote_io.py", line 237, in <genexpr>
    self.assertEqual(sum(1 for _ in s3_lister_dp), input[1], f"{input[0]} failed")
  File "/shared/erjia/pytorch/torch/utils/data/datapipes/_typing.py", line 501, in wrap_generator
    response = gen.send(None)
  File "/shared/erjia/data/torchdata/datapipes/iter/load/s3io.py", line 64, in __iter__
    urls = self.handler.list_files(prefix)
ValueError: Access Denied
This exception is thrown by __iter__ of S3FileListerIterDataPipe(length=-1, source_datapipe=IterableWrapperIterDataPipe)

----------------------------------------------------------------------
Ran 1 test in 0.502s

FAILED (errors=1)
srun: error: a100-st-p4d24xlarge-1: task 0: Exited with exit code 1

Let me create a script and run gdb for you.

So the error still occurs when trying to list_files(prefix). Could you check what $HOME and $AWS_CONFIG_FILE is and whether there's a $HOME/.aws/config file. If so, what's the content of it?

ejguan

Thank you so much for adding this fix so quickly. LGTM.

torchdata/datapipes/iter/load/s3io.py

ejguan · 2022-05-25T19:23:05Z

And, based on our offline discussion, it seems I don't have permission to run configuration for aws on our AWS cluster. So, the Error on my local environment is not related to the implementation.

ydaiming · 2022-05-25T19:36:06Z

@ejguan updated. Please review. Thanks.

ejguan · 2022-05-25T19:49:56Z

torchdata/datapipes/iter/load/README.md

 pip uninstall torchdata -y
+pip uninstall torchdata -y
+git clone https://github.com/pytorch/data.git
+cd data
 python setup.py clean
-python setup.py install
+BUILD_S3=1 python setup.py install


nit: duplicate uninstall

Could you please also add this link https://github.com/pytorch/data/tree/main/torchdata/datapipes/iter/load#installation to https://github.com/pytorch/data/blob/main/README.md (in from source section) to let users know they can install with AWSSDK enabled.

ejguan · 2022-05-25T21:03:42Z

README.md

@@ -101,6 +101,8 @@ assert batch['text'][0][0:8] == ['Wall', 'St.', 'Bears', 'Claw', 'Back', 'Into',
 python setup.py install
 ```

+In you'd like to include the S3 IO datapipes and aws-sdk-cpp, you may also follow [the instructions here](https://github.com/pytorch/data/blob/1d1cdbefe38041e067cda835ced9b5a5c59b3e5b/torchdata/datapipes/iter/load/README.md#build-from-source)


Could you please use https://github.com/pytorch/data/blob/main/torchdata/datapipes/iter/load/README.md to always link it to the main branch?

facebook-github-bot · 2022-05-25T21:14:51Z

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

Summary: Please read through our [contribution guide](https://github.com/pytorch/data/blob/main/CONTRIBUTING.md) prior to creating your pull request. - Note that there is a section on requirements related to adding a new DataPipe. Fixes pytorch#460 ### Changes - update the s3 test cases due to an update in the public dataset Pull Request resolved: pytorch#464 Reviewed By: NivekT Differential Revision: D36678285 Pulled By: ejguan fbshipit-source-id: 5e579d20290aec0f4f0a17031fe1bb256a640231

Summary: Please read through our [contribution guide](https://github.com/pytorch/data/blob/main/CONTRIBUTING.md) prior to creating your pull request. - Note that there is a section on requirements related to adding a new DataPipe. Fixes #460 ### Changes - update the s3 test cases due to an update in the public dataset Pull Request resolved: #464 Reviewed By: NivekT Differential Revision: D36678285 Pulled By: ejguan fbshipit-source-id: 5e579d20290aec0f4f0a17031fe1bb256a640231

update s3 test cases

fe3750d

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label May 25, 2022

enable datapipes test

1f44976

ejguan approved these changes May 25, 2022

View reviewed changes

torchdata/datapipes/iter/load/s3io.py Outdated Show resolved Hide resolved

revert changes in s3io.py

e94ad00

ejguan reviewed May 25, 2022

View reviewed changes

ydaiming force-pushed the fix-s3-test branch from 1d1cdbe to 9e1f152 Compare May 25, 2022 20:28

ejguan reviewed May 25, 2022

View reviewed changes

update S3 IO readme

35fe473

ydaiming force-pushed the fix-s3-test branch from 9e1f152 to 35fe473 Compare May 25, 2022 21:04

NivekT approved these changes May 25, 2022

View reviewed changes

ejguan linked an issue May 25, 2022 that may be closed by this pull request

The developer install instruction are outdated #432

Closed

fix lint sytle

ced6c63

ydaiming force-pushed the fix-s3-test branch from f6fa592 to ced6c63 Compare May 26, 2022 03:09

facebook-github-bot closed this in 62fe510 May 26, 2022

ejguan mentioned this pull request May 26, 2022

[v0.4.0] Release Tracker #448

Closed

ejguan mentioned this pull request May 26, 2022

[Release 0.4.0] update s3 test cases (#464) #475

Merged

NivekT mentioned this pull request Jun 10, 2022

The developer install instruction are outdated #432

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update s3 test cases #464

update s3 test cases #464

ydaiming commented May 25, 2022 •

edited

ydaiming commented May 25, 2022

ejguan commented May 25, 2022

ydaiming commented May 25, 2022

ejguan commented May 25, 2022

ejguan commented May 25, 2022

ydaiming commented May 25, 2022

ejguan commented May 25, 2022

ydaiming commented May 25, 2022

ejguan commented May 25, 2022

ydaiming commented May 25, 2022 •

edited

ydaiming commented May 25, 2022

ejguan commented May 25, 2022

ydaiming commented May 25, 2022

ejguan left a comment

ejguan commented May 25, 2022

ydaiming commented May 25, 2022

ejguan May 25, 2022

ydaiming May 25, 2022

ejguan May 25, 2022

ydaiming May 25, 2022

facebook-github-bot commented May 25, 2022

update s3 test cases #464

update s3 test cases #464

Conversation

ydaiming commented May 25, 2022 • edited

Changes

ydaiming commented May 25, 2022

ejguan commented May 25, 2022

ydaiming commented May 25, 2022

ejguan commented May 25, 2022

ejguan commented May 25, 2022

ydaiming commented May 25, 2022

ejguan commented May 25, 2022

ydaiming commented May 25, 2022

ejguan commented May 25, 2022

ydaiming commented May 25, 2022 • edited

ydaiming commented May 25, 2022

ejguan commented May 25, 2022

ydaiming commented May 25, 2022

ejguan left a comment

Choose a reason for hiding this comment

ejguan commented May 25, 2022

ydaiming commented May 25, 2022

ejguan May 25, 2022

Choose a reason for hiding this comment

ydaiming May 25, 2022

Choose a reason for hiding this comment

ejguan May 25, 2022

Choose a reason for hiding this comment

ydaiming May 25, 2022

Choose a reason for hiding this comment

facebook-github-bot commented May 25, 2022

ydaiming commented May 25, 2022 •

edited

ydaiming commented May 25, 2022 •

edited