Introduce OutputDoc by klecki · Pull Request #1828 · NVIDIA/DALI

klecki · 2020-03-18T18:24:58Z

Why we need this PR?

We want better docs

What happened in this PR?

What solution was applied:
[
Introduce OutputDoc and OutputDocStr as a mechanism for creating Returns section for Operator __call__, similar to InputDoc & InputDocString that were already present.
Fill it for most operators with non-trivial returns.
]
Affected modules and functionalities:
[
OpSchema, ops.py - docstring generator, docs of some Ops
]
Key points relevant for the review:
[
Please suggest how to better document shapes and operators with variable number of outputs.
]
Validation and testing:
[ Needs some proofreading mostly and looking through generated docs ]
Documentation (including examples):
[ Most certainly ]

JIRA TASK: [Use DALI-XXXX or NA]

klecki · 2020-03-18T18:25:05Z

!build

dali-automaton · 2020-03-18T18:31:23Z

CI MESSAGE: [1197587]: BUILD STARTED

mzient · 2020-03-18T18:42:20Z

Suggested change

.OutputDoc(1, "sampling_rage", "TensorList of float", "Batch of sampling rates [Hz]")

.OutputDoc(1, "sampling_rate", "TensorList of float", "The sampling rates corresponding to the decoded sound recordings [Hz]")

mzient · 2020-03-18T18:44:36Z

Suggested change

.OutputDoc(0, "decoded", "TensorList of int16, int32 or float", "Batch of decoded data")

.OutputDoc(0, "decoded", "TensorList of int16, int32 or float", "The decoded audio recordings")

mzient · 2020-03-18T18:45:17Z

Suggested change

.OutputDoc(0, "start", "TensorList of int", "Begin indices of nonsilent regions")

.OutputDoc(0, "start", "TensorList of int", "Start positions, in samples, of nonsilent regions")

mzient · 2020-03-18T18:45:30Z

Suggested change

.OutputDoc(1, "length", "TensorList of int", "Lengths of nonsilent regions")

.OutputDoc(1, "length", "TensorList of int", "Lengths, in samples, of nonsilent regions")

mzient · 2020-03-18T18:47:31Z

Suggested change

.OutputDoc(0, "images", "1D TensorList of uint8", "Undecoded image data")

.OutputDoc(0, "data", "1D TensorList of uint8", "Raw file contents")

This reader can also be used for audio or whatever else...

Right. Probably same with others.

mzient · 2020-03-18T18:48:35Z

Suggested change

.OutputDoc(0, "images", "1D TensorList of uint8", "Undecoded image data")

.OutputDoc(0, "data", "1D TensorList of uint8", "Raw data buffers")

mzient · 2020-03-18T18:49:46Z

Yeah, I will remove it, I'm still working on some parts and wanted to render the docs for everyone to see.

mzient · 2020-03-18T18:51:58Z

This monster should go back to the fiery chasm from whence it came ;)

mzient · 2020-03-18T18:54:43Z

dali-automaton · 2020-03-18T20:48:14Z

CI MESSAGE: [1197587]: BUILD PASSED

JanuszL · 2020-03-18T23:19:06Z

Suggested change

"Output dox cannot be used when the AdditionalOutputFn was set");

"Output doc cannot be used when the AdditionalOutputFn was set");

jantonguirao · 2020-03-19T13:53:17Z

this will change as soon as I merge my BBox PR. I am no longer using the ltrb terminology, and now you can have an arbitrary order of the coordinates

I will wait for your PR, and replace those docs, it's basically placeholder taken from the docstring (as is in other places...).

jantonguirao · 2020-03-19T13:53:41Z

about to change as we'll support 3d as well (same below)

Took changes from your PR.

jantonguirao · 2020-03-19T13:54:02Z

Suggested change

.InputDoc(0, "BBoxes", "TensorList of floats",

.InputDoc(0, "bboxes", "TensorList of floats",

Either all lowercase or all capitalized

jantonguirao · 2020-03-19T13:55:03Z

I think it's {batch_size, m}

Nope,

void RandomBBoxCrop<CPUBackend>::WriteLabelsToOutput( SampleWorkspace &ws, const std::vector<int> &labels) const { auto &labels_out = ws.Output<CPUBackend>(3); labels_out.Resize({static_cast<Index>(labels.size()), 1});

But it still more like batch * {m_i, 1}, where m_i is different for every sample i

jantonguirao · 2020-03-19T13:55:22Z

4 will change. Also, it's not clear what m stands for

I tried to rephrase it better. m was taken from the current description that was just using it. I'm also not sure what it stands for :P

jantonguirao · 2020-03-19T13:56:11Z

Suggested change

"If provided, the output is given the same shape as `data` and the contents are ignored")

"If provided, the output is given the same shape as `data` (data contents are ignored)")

jantonguirao · 2020-03-19T13:56:31Z

Suggested change

undecoded image data, only if image_available = true

encoded image data, only if image_available = true

jantonguirao · 2020-03-19T13:58:26Z

I'd write "optical flow" instead of OF

the rest of the doc already use OF extensively.

Ok then, but I don't like using "OF" as if it was a well-established acronym

jantonguirao · 2020-03-19T13:59:18Z

same comments as before

jantonguirao · 2020-03-19T14:00:52Z

Suggested change

std::string name = {};

std::string name;

isn't it the same?

jantonguirao · 2020-03-19T14:01:39Z

Suggested change

std::vector<InOutDoc> input_dox_ = {};

std::vector<InOutDoc> input_dox_;

?

jantonguirao · 2020-03-19T14:01:49Z

klecki · 2020-03-20T16:27:56Z

!build

klecki · 2020-03-20T16:28:15Z

Waits for #1785 to stabilize the docs.

dali-automaton · 2020-03-20T16:37:15Z

CI MESSAGE: [1202805]: BUILD STARTED

dali-automaton · 2020-03-20T18:39:42Z

CI MESSAGE: [1202805]: BUILD PASSED

…eturns Introduce OutputDoc and OutputDocStr as a mechanism for creating Returns section for Operator __call__, similar to InputDoc & InputDocString that were already present. Allow for unnamed outputs Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>

klecki · 2020-03-26T15:10:30Z

!build

dali-automaton · 2020-03-26T15:15:33Z

CI MESSAGE: [1214698]: BUILD STARTED

dali-automaton · 2020-03-26T19:19:32Z

CI MESSAGE: [1214698]: BUILD PASSED

jantonguirao · 2020-04-10T07:09:30Z

 )code")
  .NumInput(1)
  .NumOutput(2)
+  .OutputDoc(0, "decoded", "TensorList of int16, int32 or float", "The decoded audio recordings.")


I think I remember the conclusion was to start using TensorList for describing outputs. Wasn't it?

jantonguirao · 2020-04-10T07:14:31Z

+``{m_i ,4}`` Tensor (``m_i * [x, y, w, h]`` or ``m_i * [left, top, right, bottom]``)
+and labels as ``{m_i, 1}`` Tensor (``m * category_id``).)code")
+.OutputDocStr(R"code(images : 1D TensorList of uint8
+    Encoded image data.s.


What is data.s.?

klecki · 2020-05-14T12:37:02Z

Continued in #1908

klecki changed the title ~~Introduce OutputDoc and fill it for most operator with non-trivaial returns~~ Introduce OutputDoc and fill it for most operators with non-trivial returns Mar 18, 2020

klecki requested review from JanuszL, awolant, banasraf, jantonguirao and szalpal March 18, 2020 18:26

mzient reviewed Mar 18, 2020

View reviewed changes

Comment thread dali/pipeline/operator/op_schema.cc Outdated

mzient Mar 18, 2020

Copy link
Copy Markdown

Contributor

Choose a reason for hiding this comment

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

👍

JanuszL reviewed Mar 18, 2020

View reviewed changes

jantonguirao reviewed Mar 19, 2020

View reviewed changes

klecki force-pushed the output-dox branch from 41ce3bd to 7b7dbf8 Compare March 26, 2020 15:07

klecki changed the title ~~Introduce OutputDoc and fill it for most operators with non-trivial returns~~ Introduce OutputDoc Mar 26, 2020

jantonguirao reviewed Apr 10, 2020

View reviewed changes

jantonguirao approved these changes Apr 10, 2020

View reviewed changes

klecki marked this pull request as draft April 22, 2020 10:54

klecki mentioned this pull request Apr 27, 2020

Add output docs #1908

Draft

3 tasks

klecki closed this May 14, 2020

	.OutputDoc(1, "sampling_rage", "TensorList of float", "Batch of sampling rates [Hz]")
	.OutputDoc(1, "sampling_rate", "TensorList of float", "The sampling rates corresponding to the decoded sound recordings [Hz]")

	.OutputDoc(0, "decoded", "TensorList of int16, int32 or float", "Batch of decoded data")
	.OutputDoc(0, "decoded", "TensorList of int16, int32 or float", "The decoded audio recordings")

	.OutputDoc(0, "start", "TensorList of int", "Begin indices of nonsilent regions")
	.OutputDoc(0, "start", "TensorList of int", "Start positions, in samples, of nonsilent regions")

	.OutputDoc(1, "length", "TensorList of int", "Lengths of nonsilent regions")
	.OutputDoc(1, "length", "TensorList of int", "Lengths, in samples, of nonsilent regions")

	.OutputDoc(0, "images", "1D TensorList of uint8", "Undecoded image data")
	.OutputDoc(0, "data", "1D TensorList of uint8", "Raw file contents")

	"Output dox cannot be used when the AdditionalOutputFn was set");
	"Output doc cannot be used when the AdditionalOutputFn was set");

	.InputDoc(0, "BBoxes", "TensorList of floats",
	.InputDoc(0, "bboxes", "TensorList of floats",

	"If provided, the output is given the same shape as `data` and the contents are ignored")
	"If provided, the output is given the same shape as `data` (data contents are ignored)")

	undecoded image data, only if image_available = true
	encoded image data, only if image_available = true

	std::vector<InOutDoc> input_dox_ = {};
	std::vector<InOutDoc> input_dox_;

Conversation

klecki commented Mar 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why we need this PR?

What happened in this PR?

Uh oh!

klecki commented Mar 18, 2020

Uh oh!

dali-automaton commented Mar 18, 2020

Uh oh!

mzient Mar 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mzient Mar 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mzient Mar 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dali-automaton commented Mar 18, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

klecki commented Mar 18, 2020 •

edited

Loading

mzient Mar 18, 2020 •

edited

Loading

mzient Mar 18, 2020 •

edited

Loading

mzient Mar 18, 2020 •

edited

Loading

jantonguirao Apr 10, 2020 •

edited

Loading