Improved NDD skills

[mzient]

Category:

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

Description:

Improved NDD skills

• Add advanced slicing.
• Use .tensors[] rather than .select()
• Mention that spelling out default values has a nontrivial cost.

Additional information:

Affected modules and functionalities:

Key points relevant for the review:

Tests:

• Existing tests apply
• 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

[greptile-apps]

Greptile Summary

This PR updates the DALI NDD (No-Defined-Pipeline Dynamic mode) skill documentation and example notebooks to migrate the deprecated .select() API to the new .tensors[] subscript API, adds an advanced slicing section with per-sample batch-of-indices support, and introduces a performance note about the cost of spelling out default argument values.

• API migration: All occurrences of batch.select(i) in SKILL.md, evals.json, and eight example notebooks are updated to batch.tensors[i], aligning the documentation with the current NDD public API.
• New content in SKILL.md: An "Advanced slicing" subsection shows how .slice[] accepts a mix of scalar and per-sample batch indices (e.g., imgs.slice[42 : ndd.batch(imgs.shape).slice[0] // 2]), and a new "Common mistakes" row warns that explicitly spelling out default argument values incurs non-trivial Python overhead because it bypasses the fast-path sentinel check.
• Eval assertions updated: Both evals.json assertions that referenced batch.select() are updated to reference batch.tensors[] to remain aligned with the skill they test.

Confidence Score: 5/5

Pure documentation and skill update — no runtime or functional code changes; safe to merge.

All changes are confined to Markdown skill documentation, eval assertion strings, and Jupyter notebook cells. Every .select() call has a consistent mechanical replacement with .tensors[], the new advanced-slicing example matches the documented API, and the eval assertions remain logically correct after the update. No logic, data flow, or compiled code is touched.

No files require special attention.

Important Files Changed

Filename	Overview
skills/dali-dynamic-mode/SKILL.md	Migrates .select() → .tensors[], adds Advanced Slicing section and a default-args performance row in the mistakes table. Content is internally consistent.
skills/dali-dynamic-mode/evals/evals.json	Updates two assertion strings to reference batch.tensors[i] instead of the deprecated batch.select(i), keeping eval assertions consistent with the updated skill.
docs/examples/getting_started/dynamic_mode.ipynb	Three .select() → .tensors[] replacements including the warning prose and visualization loop; changes are mechanical and correct.
docs/examples/general/data_loading/coco_reader/dynamic_mode.ipynb	Six .select() → .tensors[] replacements across bbox/label/polygon inspection and plot_sample helper; all correct.
docs/examples/image_processing/clahe/dynamic_mode.ipynb	Five .select() → .tensors[] replacements in MRI batch construction and contrast analysis loops; all correct.
docs/examples/image_processing/resize/dynamic_mode.ipynb	Single .select() → .tensors[] change in image decoding chain; correct.
docs/examples/sequence_processing/video/video_file_list_outputs/dynamic_mode.ipynb	Four .select() → .tensors[] replacements for video/label/frame/timestamp batch access; all correct.
docs/examples/sequence_processing/video/video_reader_label/dynamic_mode.ipynb	Single .select() → .tensors[] replacement for video/label sample access; correct.
docs/examples/sequence_processing/video/video_reader_simple/dynamic_mode.ipynb	Single .select() → .tensors[] replacement for video sample access; correct.
docs/examples/image_processing/decoder/dynamic_mode.ipynb	Single .select() → .tensors[] replacement in image batch visualization loop; correct.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Batch] --> B{Intent}
    B -->|Select specific sample| C["batch.tensors[i] → Tensor"]
    B -->|Select subset of samples| D["batch.tensors[slice or list] → Batch"]
    B -->|Slice within every sample with scalar bounds| E["batch.slice[start:stop] → Batch"]
    B -->|Slice within every sample with per-sample bounds| F["batch.slice[batch_start : batch_stop] → Batch"]
    C --> G["Deprecated: batch.select(i)"]
    style G fill:#ffcccc

Loading

_{Reviews (5): Last reviewed commit: "Explain how skipping arguments avoids py..." | Re-trigger Greptile}

[greptile-apps]

[Bug] Float division produces float slice bounds here. Batch.__truediv__ (i.e. /) is wired to fdiv — floating-point division — so ndd.batch(imgs.shape).slice[0] / 2 yields a Batch of floats (e.g. 240.0, 360.0). DALI's TensorSubscript will either reject those float bounds or interpret them as normalised coordinates rather than integer pixel offsets. Replace with // 2 (Batch.__floordiv__ → div, integer-preserving) to match the stated intent of "half of the height".

Suggested change

	ndd.batch(imgs.shape).slice[0] / 2 # half of the height of _respective_ samples
	ndd.batch(imgs.shape).slice[0] // 2 # half of the height of _respective_ samples

[mzient]

@greptileai It's an antipattern that needs fixing, but the code does work as written and produces correct results.

[greptile-apps]

Fair enough — if DALI coerces float slice bounds to integers internally and produces the correct result, then it's not a bug. I'll withdraw the "Bug" label.

That said, using / 2 instead of // 2 is still worth a nudge in docs/examples, since example code tends to be copied verbatim and the implicit coercion isn't obvious to readers.

Shall I remember a rule: "Don't flag float-to-int coercion in DALI slice bounds as a bug — it works correctly, though // is preferred style in examples"?

[jantonguirao]

[Minor / clarity] Two things worth tightening on this example line:

1. ndd.imread already handles the list-of-paths case by returning a Batch directly (per _imread.py — single path → Tensor, list → Batch). The inner list-comp + ndd.as_batch wrap is redundant; imgs = ndd.imread(filenames) is equivalent and reads more like idiomatic ndd.
2. ndd.imread is not introduced anywhere else in this skill. A reader (or LLM consuming the skill) seeing it here for the first time has to guess what it does. Either add a one-line note where it appears, or use a more familiar primitive that the skill already covers.

[jantonguirao]

[Nit / clarity] The comment # value broadcast to all samples is hard to parse — it doesn't say what value or what it contrasts with. The point of this snippet (per the section heading and prose above) is that .slice[] accepts a mix of scalars and per-sample batches; that contrast only lands if the two comments are parallel. Consider e.g.:

    42 :  # scalar start (same for all samples)
    ndd.batch(imgs.shape).slice[0] // 2  # per-sample stop (half height of each)

[jantonguirao]

[Minor / consistency] This is now the only place in the file that still mentions .select() as a sample-picking API. The PR consistently migrates everywhere else (line 41-42 table, line 46 prose, line 250 migration row) to .tensors[] only — a reader or LLM trained on the skill will pick up the inconsistency and may emit .select() calls again.

If .select() still exists for backward compat, consider stating that explicitly somewhere (e.g. ".select() is the legacy spelling") rather than slipping it into the explanation here. Otherwise it's simpler to drop it: `.tensors[]` picks samples; `.slice` slices within each sample.

[jantonguirao]

[Minor / actionability] The Why cell asserts "Very high Python-side overhead, especially when the argument accepts Tensors/Batches" without telling the reader why — in most Python libraries, passing a default explicitly is equivalent to omission, so this rule reads as surprising. Without a mechanism, a reader can't tell when the warning generalizes (e.g. is this true for plain scalar defaults too, or only Tensor/Batch-typed ones?).

One extra clause would make this self-justifying, e.g. "unset args are handled efficiently by the backend via sentinels; passing the default explicitly forces Python-side conversion — costly for Tensor/Batch types."

[JanuszL]

@mzient please rebase and then we need to issue /nvskills-ci to rereview and sign again.

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[JanuszL]

/nvskills-ci