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

Fixes ExternalSource for the GPU #1452

Merged
merged 1 commit into from
Nov 28, 2019
Merged

Fixes ExternalSource for the GPU #1452

merged 1 commit into from
Nov 28, 2019

Conversation

JanuszL
Copy link
Contributor

@JanuszL JanuszL commented Nov 7, 2019

  • fixes handling of feed_input method when ExternalSource is located
    in the GPU
  • updates the ExternalSource example by layout usage
  • adds a notice that feed_input should be called from the inside of
    iter_setup method

Signed-off-by: Janusz Lisiecki jlisiecki@nvidia.com

Why we need this PR?

  • fixes handling of feed_input method when ExternalSource is located in the GPU
  • updates the ExternalSource example by layout usage
  • adds a notice that feed_input should be called from the inside of iter_setup method

What happened in this PR?

JIRA TASK: [NA]

@JanuszL
Copy link
Contributor Author

JanuszL commented Nov 7, 2019

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [980233]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [980233]: BUILD FAILED

@JanuszL
Copy link
Contributor Author

JanuszL commented Nov 8, 2019

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [981840]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [981840]: BUILD FAILED

@JanuszL
Copy link
Contributor Author

JanuszL commented Nov 8, 2019

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [981939]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [981939]: BUILD PASSED

super(ExternalSourcePipeline, self).__init__(batch_size, num_threads, device_id)
self.input = ops.ExternalSource(device="gpu")
self.crop = ops.Crop(device="gpu", crop_h=32, crop_w=32, crop_pos_x=0.2, crop_pos_y=0.2)
#self.batch_size = batch_size
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
#self.batch_size = batch_size

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Leftover. Done

@JanuszL
Copy link
Contributor Author

JanuszL commented Nov 13, 2019

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [989683]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [989683]: BUILD PASSED

DALI_ENFORCE(source != nullptr, "Input name '" +
name + "' is not marked as an external input.");
source->SetDataSource(tl);
OperatorBase *op_ptr = &node.InstantiateOperator();
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this happen before we Build the pipeline? If so, that's weird. I would just access the instance of the node, and require the Pipeline to already be built. But I see that it was already done this way. :(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you asking about 163?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And reverted as our test relies on the behavior.

except RuntimeError:
pass
else:
assert False, "ExternalSource should throw"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you use nose.assert_raises?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

@klecki klecki left a comment

Choose a reason for hiding this comment

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

Do you somehow check the weird combinations of ExternalSource(device="cpu") that is fed gpu data and vice-versa?

@JanuszL
Copy link
Contributor Author

JanuszL commented Nov 22, 2019

Do you somehow check the weird combinations of ExternalSource(device="cpu") that is fed gpu data and vice-versa?

We test CPU data to the GPU in test_external_source_gpu, another way around is not supported yet and probably won't be. I can imagine the only GPU to GPU but this is also not supported.

@JanuszL
Copy link
Contributor Author

JanuszL commented Nov 22, 2019

!build

@JanuszL JanuszL changed the title Fixes ExternSource for the GPU Fixes ExternalSource for the GPU Nov 22, 2019
@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1003214]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1003215]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1003230]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1003214]: BUILD FAILED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1003230]: BUILD FAILED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1003215]: BUILD FAILED

@JanuszL
Copy link
Contributor Author

JanuszL commented Nov 26, 2019

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1008359]: BUILD STARTED

@JanuszL
Copy link
Contributor Author

JanuszL commented Nov 26, 2019

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1008391]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1008391]: BUILD PASSED

iterator = iter(TestIterator(iter_num))
pipe = IterSetupPipeline(iterator, 3, 0, True)
pipe.build()
assert_raises(RuntimeError, pipe.run)
Copy link
Contributor

Choose a reason for hiding this comment

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

seems that this belongs to test_external_source_fail case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

- fixes handling of feed_input method when ExternalSource is located
  in the GPU
- updates the ExternalSource example by layout usage
- adds a notice that `feed_input` should be called from the inside of
  `iter_setup` method

Signed-off-by: Janusz Lisiecki <jlisiecki@nvidia.com>
@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1011290]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1011290]: BUILD FAILED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1011290]: BUILD PASSED

@JanuszL JanuszL merged commit 45a3ca9 into NVIDIA:master Nov 28, 2019
@JanuszL JanuszL deleted the fix_external_souorce_for_gpu branch November 28, 2019 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants