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

Fix missing source info in the numpy reader output #4714

Merged
merged 2 commits into from
Mar 22, 2023

Conversation

JanuszL
Copy link
Contributor

@JanuszL JanuszL commented Mar 15, 2023

  • fixes missing copy of the source info metadata from
    samples read in the numpy reader (CPU and GPU) to the
    operator output

Category:

Other (e.g. Documentation, Tests, Configuration)

Description:

  • fixes missing copy of the source info metadata from
    samples read in the numpy reader (CPU and GPU) to the
    operator output

Additional information:

Affected modules and functionalities:

  • numpy reader (CPU and GPU) operator

Key points relevant for the review:

  • NA

Tests:

  • Existing tests apply
    • test_numpy.test_pad_last_sample (updated it)
  • New tests added
    • Python tests
    • GTests
    • Benchmark
    • Other
  • N/A

Checklist

Documentation

  • Existing documentation applies
  • Documentation updated
    • Docstring
    • Doxygen
    • RST
    • Jupyter
    • Other
  • N/A

DALI team only

Requirements

  • Implements new requirements
  • Affects existing requirements
  • N/A

REQ IDs: N/A

JIRA TASK: N/A

@JanuszL
Copy link
Contributor Author

JanuszL commented Mar 15, 2023

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [7594045]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [7594045]: BUILD FAILED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [7594045]: BUILD PASSED

@@ -658,13 +669,10 @@ def check_pad_last_sample(device):
for _ in range(2):
pipe_out = pipe.run()
for i in range(batch_size):
pipe_arr = to_array(pipe_out[0][i])
out_arr = to_array(pipe_out[0][i])
out_prop = to_array(pipe_out[1][i])
Copy link
Contributor

@mzient mzient Mar 22, 2023

Choose a reason for hiding this comment

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

SourceInfo has a first-class support, so you could just do:

Suggested change
out_prop = to_array(pipe_out[1][i])
out_prop = pipe_out[0][i].source_info()

This way there wouldn't be a need to use get_property() in the pipeline.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

- fixes missing copy of the source info metadata from
  samples read in the numpy reader (CPU and GPU) to the
  operator output

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

@mzient mzient left a comment

Choose a reason for hiding this comment

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

With optional improvements.

Signed-off-by: Janusz Lisiecki <jlisiecki@nvidia.com>
@JanuszL
Copy link
Contributor Author

JanuszL commented Mar 22, 2023

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [7673596]: BUILD STARTED

def check_pad_last_sample(device):
@params('cpu', 'gpu')
def test_pad_last_sample(device):
def uint8_tensor_to_string(t):

Check notice

Code scanning / CodeQL

Unused local variable

Variable uint8_tensor_to_string is not used.
@dali-automaton
Copy link
Collaborator

CI MESSAGE: [7673596]: BUILD FAILED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [7673596]: BUILD PASSED

@JanuszL JanuszL merged commit 51a5633 into NVIDIA:main Mar 22, 2023
@JanuszL JanuszL deleted the fix_numpy_source_inf branch March 22, 2023 18:28
stiepan pushed a commit to stiepan/DALI that referenced this pull request Mar 23, 2023
- fixes missing copy of the source info metadata from
  samples read in the numpy reader (CPU and GPU) to the
  operator output

Signed-off-by: Janusz Lisiecki <jlisiecki@nvidia.com>
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.

4 participants