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

Expose names of Pipeline outputs in C API #2665

Merged
merged 4 commits into from Feb 10, 2021
Merged

Conversation

szalpal
Copy link
Member

@szalpal szalpal commented Feb 9, 2021

Signed-off-by: szalpal mszolucha@nvidia.com

Why we need this PR?

Pick one, remove the rest

  • It adds new feature needed because we need names of the outputs to map them in dali_backend

What happened in this PR?

Fill relevant points, put NA otherwise. Replace anything inside []

  • What solution was applied:
    augment C API
  • Affected modules and functionalities:
    C API, Pipeline class
  • Key points relevant for the review:
    NA
  • Validation and testing:
    Yes
  • Documentation (including examples):
    Yes

JIRA TASK: [Use DALI-XXXX or NA]

Signed-off-by: szalpal <mszolucha@nvidia.com>
@szalpal
Copy link
Member Author

szalpal commented Feb 9, 2021

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [2058116]: BUILD STARTED

Signed-off-by: szalpal <mszolucha@nvidia.com>
@szalpal
Copy link
Member Author

szalpal commented Feb 9, 2021

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [2058174]: BUILD STARTED

@mzient mzient requested a review from a team February 9, 2021 08:41
@@ -97,7 +98,7 @@ std::unique_ptr<Pipeline> GetTestPipeline(bool is_file_reader, const std::string
.AddInput(input_name, exec_device)
.AddOutput("outputs", exec_device));
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.AddOutput("outputs", exec_device));
.AddOutput(output_name, exec_device));

Signed-off-by: szalpal <mszolucha@nvidia.com>
@@ -335,6 +335,11 @@ DLL_PUBLIC size_t daliMaxDimTensors(daliPipelineHandle *pipe_handle, int n);
*/
DLL_PUBLIC unsigned daliGetNumOutput(daliPipelineHandle *pipe_handle);

/**
* @brief Returns a string indicating name of the output given by id
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it make sense to mention that the returned pointer is valid as long as the pipeline exists?

device_id, false, prefetch_queue_depth, prefetch_queue_depth,
prefetch_queue_depth, false);

ASSERT_EQ(daliGetNumOutput(&handle), 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it would be better to check more than one output in this test.

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [2058174]: BUILD PASSED

Signed-off-by: szalpal <mszolucha@nvidia.com>
@szalpal
Copy link
Member Author

szalpal commented Feb 10, 2021

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [2062493]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [2062493]: BUILD PASSED

@szalpal szalpal merged commit 0ba4b2d into NVIDIA:master Feb 10, 2021
@szalpal szalpal deleted the named_tensors branch February 9, 2024 00:25
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.

None yet

4 participants