Add ExternalSource to dynamic mode#6395
Conversation
There was a problem hiding this comment.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
|
| Filename | Overview |
|---|---|
| dali/python/nvidia/dali/experimental/dynamic/_external_source.py | New ExternalSource class wrapping get_callback_from_source for dynamic mode; logic for num_outputs, cycle, layout/dtype broadcasting, and Batch vs Tensor dispatch is correct. |
| dali/test/python/experimental_mode/test_external_source.py | Good coverage of callable/iterable/generator sources, batch broadcast, multi-output, cycle modes, layout/dtype, and GPU; missing an assert_raises test for num_outputs <= 0 constructor validation. |
| dali/python/nvidia/dali/experimental/dynamic/init.py | One-line export addition for ExternalSource; no issues. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["ExternalSource.__call__(batch_size?)"] --> B["_callback() ← get_callback_from_source"]
B --> C["_get_outputs(data)"]
C -->|"num_outputs == 1"| D["wrap data in 1-tuple"]
C -->|"num_outputs > 1"| E{data is Sequence of correct length?}
E -->|No| F["ValueError: expected N outputs"]
E -->|Yes| G["return data tuple"]
D & G --> H["for each output: _convert_output(data, batch_size, idx)"]
H --> I{_get_batch_size not None?}
I -->|"Yes (Batch / TensorList)"| J["as_batch(...) + validate batch_size"]
I -->|"No (scalar tensor)"| K["as_tensor(...)"]
K -->|"batch_size given"| L["Batch.broadcast(tensor, batch_size)"]
K -->|"no batch_size"| M["return Tensor"]
J --> N["return Batch"]
L --> N
N & M --> O["_are_types_uniform(results)?"]
O -->|No| P["TypeError: mixed Tensor/Batch"]
O -->|Yes| Q{num_outputs == 1?}
Q -->|Yes| R["return results[0]"]
Q -->|No| S["return results tuple"]
Reviews (3): Last reviewed commit: "Add tests for ndd.ExternalSource" | Re-trigger Greptile
9cbee37 to
9d971a3
Compare
Signed-off-by: Rostan Tabet <rtabet@nvidia.com>
Signed-off-by: Rostan Tabet <rtabet@nvidia.com>
9d971a3 to
d10ecc5
Compare
Category:
New feature (non-breaking change which adds functionality)
Description:
Add
ExternalSourceto dynamic mode. It mimicsfn.external_sourceto some extent but behaves differently, especially with batches, to integrate well with the imperative API.This is not a very useful operator in and of itself but will enable sources other than a reader in transparent pipelining.
Additional information:
Affected modules and functionalities:
Dynamic mode.
Key points relevant for the review:
Tests:
Checklist
Documentation
ndd.ExternalSourcedoesn't appear in the documentation yet because it's only really useful with transparent pipelining.DALI team only
Requirements
REQ IDs: N/A
JIRA TASK: DALI-4745