Rework frameworks notebooks to fn API #2761

awolant · 2021-03-05T01:23:05Z

Signed-off-by: Albert Wolant awolant@nvidia.com

Why we need this PR?

Refactoring to improve frameworks examples.

What happened in this PR?

What solution was applied:
Update frameworks examples notebooks to fn API
Affected modules and functionalities:
Frameworks examples
Documentation (including examples):
Frameworks examples

JIRA TASK: [DALI-1879]

Signed-off-by: Albert Wolant <awolant@nvidia.com>

review-notebook-app · 2021-03-05T01:23:09Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

awolant · 2021-03-05T01:23:14Z

!build

dali-automaton · 2021-03-05T01:25:59Z

CI MESSAGE: [2134977]: BUILD STARTED

dali-automaton · 2021-03-05T03:13:34Z

CI MESSAGE: [2134977]: BUILD PASSED

mzient · 2021-03-05T07:06:37Z

docs/examples/frameworks/mxnet/mxnet-various-readers.ipynb

@@ -96,31 +96,26 @@
   "metadata": {},


common part of pipeline, other pipelines will inherit it
There's no inheritance
There's no pipeline yet (as it used to be), so "other pipelines" doesn't make sense any more

Suggested alternative:

common part of the processing graph, used by all pipelines

Reply via ReviewNB

mzient · 2021-03-05T07:06:38Z

docs/examples/frameworks/paddle/paddle-various-readers.ipynb

@@ -96,31 +96,26 @@
   "metadata": {},


common part of pipeline, other pipelines will inherit it
There's no inheritance
There's no pipeline yet (as it used to be), so "other pipelines" doesn't make sense any more

Suggested alternative:

common part of the processing graph, used by all pipelines

Reply via ReviewNB

mzient · 2021-03-05T07:06:38Z

docs/examples/frameworks/pytorch/pytorch-various-readers.ipynb

@@ -96,31 +96,26 @@
   "metadata": {},


common part of pipeline, other pipelines will inherit it
There's no inheritance
There's no pipeline yet (as it used to be), so "other pipelines" doesn't make sense any more

Suggested alternative:

common part of the processing graph, used by all pipelines

Reply via ReviewNB

mzient · 2021-03-05T07:06:38Z

docs/examples/frameworks/tensorflow/tensorflow-various-readers.ipynb

@@ -53,7 +53,7 @@
    "tfrecord_idx = \"idx_files/train.idx\"\n",


common part of pipeline, other pipelines will inherit it
There's no inheritance
There's no pipeline yet (as it used to be), so "other pipelines" doesn't make sense any more

Suggested alternative:

common part of the processing graph, used by all pipelines

Reply via ReviewNB

jantonguirao · 2021-03-05T08:06:22Z

docs/examples/frameworks/mxnet/mxnet-various-readers.ipynb

@@ -96,31 +96,26 @@
   "metadata": {},


MXNet reader pipeline (the camel-cased name doesn't make sense now)

Reply via ReviewNB

To clarify: what you're suggesting are three separate words: MXNet reader pipeline?
+1 for that.

Done for all

jantonguirao · 2021-03-05T08:06:22Z

docs/examples/frameworks/mxnet/mxnet-various-readers.ipynb

@@ -96,31 +96,26 @@
   "metadata": {},


Caffe reader pipeline (the camel-cased name doesn't make sense now)

Reply via ReviewNB

jantonguirao · 2021-03-05T08:06:22Z

docs/examples/frameworks/mxnet/mxnet-various-readers.ipynb

@@ -96,31 +96,26 @@
   "metadata": {},


File reader pipeline (the camel-cased name doesn't make sense now)

Reply via ReviewNB

jantonguirao · 2021-03-05T08:06:23Z

docs/examples/frameworks/mxnet/mxnet-various-readers.ipynb

@@ -96,31 +96,26 @@
   "metadata": {},


TFRecord reader pipeline (the camel-cased name doesn't make sense now)

Reply via ReviewNB

jantonguirao · 2021-03-05T08:06:23Z

docs/examples/frameworks/mxnet/mxnet-various-readers.ipynb

@@ -96,31 +96,26 @@
   "metadata": {},


I guess you didn't rerun the example? (the name of the pipeline has changed)

Reply via ReviewNB

jantonguirao · 2021-03-05T08:06:24Z

docs/examples/frameworks/paddle/paddle-various-readers.ipynb

@@ -96,31 +96,26 @@
   "metadata": {},


MXNet reader pipeline

Reply via ReviewNB

jantonguirao · 2021-03-05T08:06:24Z

docs/examples/frameworks/paddle/paddle-various-readers.ipynb

@@ -96,31 +96,26 @@
   "metadata": {},


Caffe reader pipeline

Reply via ReviewNB

jantonguirao · 2021-03-05T08:06:24Z

docs/examples/frameworks/paddle/paddle-various-readers.ipynb

@@ -96,31 +96,26 @@
   "metadata": {},


File reader pipeline

Reply via ReviewNB

jantonguirao · 2021-03-05T08:06:24Z

docs/examples/frameworks/paddle/paddle-various-readers.ipynb

@@ -96,31 +96,26 @@
   "metadata": {},


TFRecord reader pipeline

Reply via ReviewNB

jantonguirao · 2021-03-05T08:06:25Z

docs/examples/frameworks/pytorch/pytorch-various-readers.ipynb

@@ -96,31 +96,26 @@
   "metadata": {},


MXNet reader pipeline

Reply via ReviewNB

jantonguirao · 2021-03-05T08:06:25Z

docs/examples/frameworks/pytorch/pytorch-various-readers.ipynb

@@ -96,31 +96,26 @@
   "metadata": {},


Caffe reader pipeline

Reply via ReviewNB

jantonguirao · 2021-03-05T08:06:25Z

docs/examples/frameworks/pytorch/pytorch-various-readers.ipynb

@@ -96,31 +96,26 @@
   "metadata": {},


File reader pipeline

Reply via ReviewNB

jantonguirao · 2021-03-05T08:06:25Z

docs/examples/frameworks/pytorch/pytorch-various-readers.ipynb

@@ -96,31 +96,26 @@
   "metadata": {},


TFRecord reader pipeline

Reply via ReviewNB

jantonguirao · 2021-03-05T08:06:27Z

docs/examples/frameworks/tensorflow/tensorflow-dataset.ipynb

@@ -21,62 +21,36 @@
   "metadata": {},


Line 28, weird indent

Reply via ReviewNB

jantonguirao · 2021-03-05T08:06:27Z

docs/examples/frameworks/tensorflow/tensorflow-dataset.ipynb

@@ -21,62 +21,36 @@
   "metadata": {},


Can we do anything about those warnings? Feel free to consider this out of scope

Reply via ReviewNB

I tried, but the suggested alternative does not work. I'll look at it separately.

review-notebook-app · 2021-03-05T08:06:28Z

View / edit / reply to this conversation on ReviewNB

jantonguirao commented on 2021-03-05T08:06:27Z
----------------------------------------------------------------

readers.MXNet -> readers.mxnet

Also, please make it a link to the documentation

awolant commented on 2021-03-05T12:31:56Z
----------------------------------------------------------------

Done

review-notebook-app · 2021-03-05T08:06:28Z

View / edit / reply to this conversation on ReviewNB

jantonguirao commented on 2021-03-05T08:06:28Z
----------------------------------------------------------------

RN50Pipeline -> rn50_pipeline

readers.MXNet -> readers.mxnet (with link)

Reader ops -> reader operators

awolant commented on 2021-03-05T12:32:01Z
----------------------------------------------------------------

Done

jantonguirao · 2021-03-05T08:06:29Z

docs/examples/frameworks/tensorflow/tensorflow-various-readers.ipynb

@@ -53,7 +53,7 @@
    "tfrecord_idx = \"idx_files/train.idx\"\n",


MXNet reader pipeline

Reply via ReviewNB

jantonguirao · 2021-03-05T08:06:29Z

docs/examples/frameworks/tensorflow/tensorflow-various-readers.ipynb

@@ -53,7 +53,7 @@
    "tfrecord_idx = \"idx_files/train.idx\"\n",


Caffe reader pipeline

Reply via ReviewNB

jantonguirao · 2021-03-05T08:06:29Z

docs/examples/frameworks/tensorflow/tensorflow-various-readers.ipynb

@@ -53,7 +53,7 @@
    "tfrecord_idx = \"idx_files/train.idx\"\n",


File reader pipeline

Reply via ReviewNB

jantonguirao · 2021-03-05T08:06:29Z

docs/examples/frameworks/tensorflow/tensorflow-various-readers.ipynb

@@ -53,7 +53,7 @@
    "tfrecord_idx = \"idx_files/train.idx\"\n",


TFRecord reader pipeline

Reply via ReviewNB

JanuszL · 2021-03-05T09:18:42Z

docs/examples/frameworks/mxnet/mxnet-various-readers.ipynb

@@ -96,31 +96,26 @@
   "metadata": {},


I think you can remove ITERATIONS at all. Now we are reading a small data set, so we don't have to exit early.

Reply via ReviewNB

JanuszL · 2021-03-05T09:18:42Z

docs/examples/frameworks/paddle/paddle-various-readers.ipynb

@@ -96,31 +96,26 @@
   "metadata": {},


I think you can remove ITERATIONS at all. Now we are reading a small data set, so we don't have to exit early.

Reply via ReviewNB

JanuszL · 2021-03-05T09:18:42Z

docs/examples/frameworks/pytorch/pytorch-various-readers.ipynb

@@ -96,31 +96,26 @@
   "metadata": {},


I think you can remove ITERATIONS at all. Now we are reading a small data set, so we don't have to exit early.

Reply via ReviewNB

JanuszL · 2021-03-05T09:19:59Z

docs/examples/frameworks/paddle/paddle-basic_example.ipynb

@@ -55,34 +55,31 @@
   "metadata": {},


I think you can remove ITERATIONS at all. Now we are reading a small data set, so we don't have to exit early.

Reply via ReviewNB

JanuszL · 2021-03-05T09:19:59Z

docs/examples/frameworks/pytorch/pytorch-basic_example.ipynb

@@ -55,34 +55,31 @@
   "metadata": {},


I think you can remove ITERATIONS at all. Now we are reading a small data set, so we don't have to exit early.

Reply via ReviewNB

…ooks

awolant · 2021-03-05T12:31:57Z

Done

View entire conversation on ReviewNB

awolant · 2021-03-05T12:32:02Z

Done

View entire conversation on ReviewNB

Signed-off-by: Albert Wolant <awolant@nvidia.com>

awolant · 2021-03-05T12:43:42Z

!build

dali-automaton · 2021-03-05T12:46:01Z

CI MESSAGE: [2136643]: BUILD STARTED

dali-automaton · 2021-03-05T14:34:45Z

CI MESSAGE: [2136643]: BUILD PASSED

Rework frameworks notebooks to fn API

1c61a0e

Signed-off-by: Albert Wolant <awolant@nvidia.com>

mzient reviewed Mar 5, 2021

View reviewed changes

jantonguirao reviewed Mar 5, 2021

View reviewed changes

jantonguirao self-assigned this Mar 5, 2021

mzient self-assigned this Mar 5, 2021

JanuszL reviewed Mar 5, 2021

View reviewed changes

Merge remote-tracking branch 'nvidia/master' into fn_frameworks_noteb…

c937c0e

…ooks

Fix for review

aa0bb09

Signed-off-by: Albert Wolant <awolant@nvidia.com>

mzient approved these changes Mar 5, 2021

View reviewed changes

jantonguirao approved these changes Mar 5, 2021

View reviewed changes

jantonguirao merged commit 72cc5a8 into NVIDIA:master Mar 5, 2021

JanuszL mentioned this pull request May 19, 2021

DALI 2021 roadmap #2978

Closed

		@@ -53,7 +53,7 @@
		"tfrecord_idx = \"idx_files/train.idx\"\n",

Rework frameworks notebooks to fn API #2761

Rework frameworks notebooks to fn API #2761

Conversation

awolant commented Mar 5, 2021

Why we need this PR?

What happened in this PR?

review-notebook-app bot commented Mar 5, 2021

awolant commented Mar 5, 2021

dali-automaton commented Mar 5, 2021

dali-automaton commented Mar 5, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

review-notebook-app bot commented Mar 5, 2021 • edited Loading

review-notebook-app bot commented Mar 5, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JanuszL Mar 5, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JanuszL Mar 5, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

awolant commented Mar 5, 2021

awolant commented Mar 5, 2021

awolant commented Mar 5, 2021

dali-automaton commented Mar 5, 2021

dali-automaton commented Mar 5, 2021

review-notebook-app bot commented Mar 5, 2021 •

edited

Loading

review-notebook-app bot commented Mar 5, 2021 •

edited

Loading

JanuszL Mar 5, 2021 •

edited

Loading

JanuszL Mar 5, 2021 •

edited

Loading