Improvements in COCO reader API #2406

jantonguirao · 2020-10-27T16:46:49Z

Why we need this PR?

Pick one, remove the rest

Refactoring to improve usability of COCOReader, especially in use cases involving segmentation masks.

What happened in this PR?

Fill relevant points, put NA otherwise. Replace anything inside []

What solution was applied:
Changed the format of mask polygon descriptor, to use indices of vertices rather than indices of coordinates
Renamed and deprecated some ambiguously named arguments
Removed trailing dimension from labels output
Added handling of mask polygons in COCO reader example
Rework the way that COCOLoader and COCOReader share data
Affected modules and functionalities:
COCOReader
Key points relevant for the review:
Changes in the COCO reader
Validation and testing:
Existing tests and jupyter example
Documentation (including examples):
COCO reader example enhanced

JIRA TASK: [DALI-1686]

Signed-off-by: Joaquin Anton <janton@nvidia.com>

jantonguirao · 2020-10-27T16:48:12Z

!build

dali-automaton · 2020-10-27T17:04:08Z

CI MESSAGE: [1737859]: BUILD STARTED

dali-automaton · 2020-10-27T18:43:29Z

CI MESSAGE: [1737859]: BUILD FAILED

Signed-off-by: Joaquin Anton <janton@nvidia.com>

dali/operators/reader/loader/coco_loader.h

jantonguirao · 2020-10-28T09:58:30Z

docs/examples/use_cases/detection_pipeline.ipynb

@@ -200,7 +200,7 @@
    "labels = labels_cpu.at(img_index)\n",
    "categories_set = set()\n",
    "for label in labels:\n",
-    "    categories_set.add(label[0])\n",
+    "    categories_set.add(label)\n",


only code where labels were expected to have an extra dim. In the detection pipelines the labels go to the box encoder, and the extra dimension is flattened there.

review-notebook-app · 2020-10-28T11:37:02Z

View / edit / reply to this conversation on ReviewNB

JanuszL commented on 2020-10-28T11:37:02Z
----------------------------------------------------------------

>Each entry in the vertices contains two coordinates (x, y)

I would say that `Each entry in the vertices contains coordinates (x, y respectively for 2D polygons).

jantonguirao · 2020-10-28T12:29:12Z

View / edit / reply to this conversation on ReviewNB

JanuszL commented on 2020-10-28T11:37:02Z

Each entry in the vertices contains two coordinates (x, y)

I would say that `Each entry in the vertices contains coordinates (x, y respectively for 2D polygons).

for 2D polygons
This gives the impression that we support other kind of polygons, which we don't. There's no 3D COCO dataset

JanuszL · 2020-10-28T12:42:53Z

dali/operators/reader/loader/coco_loader.cc

@@ -94,7 +95,7 @@ void dump_filenames(const ImageIdPairs &image_id_pairs, const std::string path)
 }

 template <typename T>
-void load_meta_file(std::vector<T> &output, const std::string path) {
+void LoadFromFile(std::vector<T> &output, const std::string path) {
  std::ifstream file(path);
  DALI_ENFORCE(file.good(), make_string("Error writing to path: ", path));


Suggested change

DALI_ENFORCE(file.good(), make_string("Error writing to path: ", path));

DALI_ENFORCE(file.good(), make_string("CocoReader meta file error while loading for path: ", path));

JanuszL · 2020-10-28T12:43:19Z

dali/operators/reader/loader/coco_loader.cc

@@ -119,7 +120,7 @@ void load_meta_file(std::vector<std::vector<T> > &output, const std::string path
  }
 }

-void load_filenames(ImageIdPairs &image_id_pairs, const std::string path) {
+void LoadFilenamesFromFile(ImageIdPairs &image_id_pairs, const std::string path) {
  std::ifstream file(path);
  DALI_ENFORCE(file, "CocoReader meta file error while loading for path: " + path);


Suggested change

DALI_ENFORCE(file, "CocoReader meta file error while loading for path: " + path);

DALI_ENFORCE(file.good(), make_string("CocoReader meta file error while loading for path: ", path));

JanuszL · 2020-10-28T12:47:23Z

dali/operators/reader/loader/coco_loader.cc

-              sample_mask_meta.push_back(objects_in_sample);
-              sample_mask_meta.push_back(obj_coords_offset + annotation.poly_.segm_meta_[i]);
-              sample_mask_meta.push_back(obj_coords_offset + annotation.poly_.segm_meta_[i + 1]);
+              auto segm_meta = annotation.poly_.segm_meta_.data();


Suggested change

auto segm_meta = annotation.poly_.segm_meta_.data();

auto &segm_meta = annotation.poly_.segm_meta_;

Signed-off-by: Joaquin Anton <janton@nvidia.com>

JanuszL · 2020-10-28T13:15:35Z

dali/operators/reader/coco_reader_op.cc

-Each mask can be one or more polygons, and for a given sample, the polygons are represented by the
-following tensors:
+  .DeprecateArg("masks", false,  // deprecated since 0.28dev
+    "``masks`` argument is now deprecated. Please use ``polygon_masks`` instead "


Maybe we should keep an info how the deprecated format looks like?

dali/operators/reader/coco_reader_op.cc

JanuszL · 2020-10-28T13:26:31Z

dali/operators/reader/coco_reader_op.cc

+images and annotation JSON files.
+
+This readers produces the following outputs::
+
+    images, bounding_boxes, labels, ((polygons, vertices) | (pixelwise_masks)), (image_ids)
+
+**images**
+
+Each sample contains image data with layout ``HWC`` (height, width, channels).
+
+**bounding_boxes**
+
+Each sample can have an arbitrary ``M`` number of bounding boxes, each described by 4 coordinates::
+
+    [[x_0, y_0, w_0, h_0],
+     [x_1, y_1, w_1, h_1]
+     ...
+     [x_M, y_M, w_M, h_M]]
+
+or in ``[l, t, r, b]`` format if requested (see ``ltrb`` argument).
+
+**labels**
+
+Each bounding box is associated with an integer label representing a category identifier::
+
+    [label_0, label_1, ..., label_M]
+
+**polygons** and **vertices** (Optional, present if ``polygon_masks`` is set to True)
+
+If ``polygon_masks`` is enabled, two extra outputs describing masks by a set of polygons.
+
+Each mask contains an arbitrary number of polygons ``P``, each associated with a mask index in the range [0, M) and 
+composed by a group of ``V`` vertices. The output ``polygons`` describes the polygons as follows::
+
+    [[mask_idx_0, start_vertex_idx_0, end_vertex_idx_0],
+     [mask_idx_1, start_vertex_idx_1, end_vertex_idx_1],
+     ...
+     [mask_idx_P, start_vertex_idx_P, end_vertex_idx_P]]
+
+where ``mask_idx`` is the index of the mask the polygon, in the range ``[0, M)``, and ``start_vertex_idx`` and  ``end_verted_idx``
+define the range of indices of vertices, as they appear in the output ``vertices``, belonging to this polygon.
+
+Each sample in ``vertices`` contains a list of vertices that composed the different polygons in the sample, as 2D coordinates::


How about making this a list:

* **images** Each sample contains image data with layout ``HWC`` (height, width, channels). * **bounding_boxes** Each sample can have an arbitrary ``M`` number of bounding boxes, each described by 4 coordinates:: [[x_0, y_0, w_0, h_0], [x_1, y_1, w_1, h_1] ... [x_M, y_M, w_M, h_M]] or in ``[l, t, r, b]`` format if requested (see ``ltrb`` argument). * **labels** Each bounding box is associated with an integer label representing a category identifier:: [label_0, label_1, ..., label_M] * **polygons** and **vertices** (Optional, present if ``polygon_masks`` is set to True) If ``polygon_masks`` is enabled, two extra outputs describing masks by a set of polygons. Each mask contains an arbitrary number of polygons ``P``, each associated with a mask index in the range [0, M) and composed by a group of ``V`` vertices. The output ``polygons`` describes the polygons as follows:: [[mask_idx_0, start_vertex_idx_0, end_vertex_idx_0], [mask_idx_1, start_vertex_idx_1, end_vertex_idx_1], ... [mask_idx_P, start_vertex_idx_P, end_vertex_idx_P]] where ``mask_idx`` is the index of the mask the polygon, in the range ``[0, M)``, and ``start_vertex_idx`` and ``end_verted_idx`` define the range of indices of vertices, as they appear in the output ``vertices``, belonging to this polygon. Each sample in ``vertices`` contains a list of vertices that composed the different polygons in the sample, as 2D coordinates:: [[x_0, y_0], [x_1, y_1], ... [x_V, y_V]] * **pixelwise_masks** (Optional, present if argument ``pixelwise_masks`` is set to True) Contains image-like data, same shape and layout as ``images``, representing a pixelwise segmentation mask. * **image_ids** (Optional, present if argument ``image_ids`` is set to True) One element per sample, representing an image identifier.

Signed-off-by: Joaquin Anton <janton@nvidia.com>

jantonguirao · 2020-10-28T18:57:14Z

!build

dali-automaton · 2020-10-28T19:00:53Z

CI MESSAGE: [1742158]: BUILD STARTED

dali-automaton · 2020-10-28T19:27:19Z

CI MESSAGE: [1742158]: BUILD FAILED

Signed-off-by: Joaquin Anton <janton@nvidia.com>

jantonguirao · 2020-10-28T19:38:35Z

!build

dali-automaton · 2020-10-28T19:45:00Z

CI MESSAGE: [1742317]: BUILD STARTED

dali-automaton · 2020-10-28T21:15:48Z

CI MESSAGE: [1742317]: BUILD FAILED

Signed-off-by: Joaquin Anton <janton@nvidia.com>

jantonguirao · 2020-10-29T09:28:40Z

!build

dali-automaton · 2020-10-29T09:36:12Z

CI MESSAGE: [1744533]: BUILD STARTED

dali-automaton · 2020-10-29T11:32:08Z

CI MESSAGE: [1744533]: BUILD PASSED

jantonguirao · 2020-10-29T14:48:30Z

dali/operators/generic/lookup_table.cc

  .AddOptionalArg("dtype",
    R"code(Output data type.)code",
-    DALI_FLOAT)
+    DALI_DATA_TYPE)


jantonguirao · 2020-10-29T14:50:10Z

!build

dali-automaton · 2020-10-29T14:55:41Z

CI MESSAGE: [1745296]: BUILD STARTED

dali-automaton · 2020-10-29T15:06:55Z

CI MESSAGE: [1745296]: BUILD FAILED

Signed-off-by: Joaquin Anton <janton@nvidia.com>

jantonguirao · 2020-10-29T15:18:06Z

!build

dali-automaton · 2020-10-29T15:21:26Z

CI MESSAGE: [1745364]: BUILD STARTED

jantonguirao · 2020-10-29T17:14:44Z

!build

dali-automaton · 2020-10-29T17:20:46Z

CI MESSAGE: [1745735]: BUILD STARTED

dali-automaton · 2020-10-29T18:59:51Z

CI MESSAGE: [1745735]: BUILD FAILED

Signed-off-by: Joaquin Anton <janton@nvidia.com>

jantonguirao · 2020-10-29T19:20:34Z

!build

dali-automaton · 2020-10-29T19:42:25Z

CI MESSAGE: [1746226]: BUILD STARTED

dali-automaton · 2020-10-30T06:44:39Z

CI MESSAGE: [1746226]: BUILD PASSED

jantonguirao added 2 commits October 27, 2020 17:45

WIP COCO reader rework

b603706

Signed-off-by: Joaquin Anton <janton@nvidia.com>

Unit tests passing now

f9098b7

Signed-off-by: Joaquin Anton <janton@nvidia.com>

Update COCO reader example

fd6b5f3

Signed-off-by: Joaquin Anton <janton@nvidia.com>

jantonguirao changed the title ~~[WIP] COCO reader rework~~ Improvements in COCO reader API Oct 28, 2020

awolant self-requested a review October 28, 2020 09:26

jantonguirao requested review from mzient and a team October 28, 2020 09:45

jantonguirao commented Oct 28, 2020

View reviewed changes

dali/operators/reader/loader/coco_loader.h Outdated Show resolved Hide resolved

jantonguirao commented Oct 28, 2020

View reviewed changes

jantonguirao force-pushed the coco_reader_rework2 branch from 879db31 to 5710dc6 Compare October 28, 2020 12:33

JanuszL reviewed Oct 28, 2020

View reviewed changes

Code review fixes

0d21763

Signed-off-by: Joaquin Anton <janton@nvidia.com>

jantonguirao force-pushed the coco_reader_rework2 branch from 5710dc6 to 0d21763 Compare October 28, 2020 12:59

JanuszL reviewed Oct 28, 2020

View reviewed changes

dali/operators/reader/coco_reader_op.cc Show resolved Hide resolved

JanuszL reviewed Oct 28, 2020

View reviewed changes

JanuszL approved these changes Oct 28, 2020

View reviewed changes

More code review changes

d073c79

Signed-off-by: Joaquin Anton <janton@nvidia.com>

jantonguirao force-pushed the coco_reader_rework2 branch from df4a273 to d073c79 Compare October 28, 2020 16:36

Merge remote-tracking branch 'upstream/master' into coco_reader_rework2

aea6a9b

Signed-off-by: Joaquin Anton <janton@nvidia.com>

jantonguirao force-pushed the coco_reader_rework2 branch from 12f5b9f to aea6a9b Compare October 28, 2020 19:38

awolant approved these changes Oct 29, 2020

View reviewed changes

Test fix

17df8de

Signed-off-by: Joaquin Anton <janton@nvidia.com>

jantonguirao force-pushed the coco_reader_rework2 branch from 5205f2e to 0fac87e Compare October 29, 2020 14:48

jantonguirao commented Oct 29, 2020

View reviewed changes

Fix deprecated arg support

aeb459f

Signed-off-by: Joaquin Anton <janton@nvidia.com>

jantonguirao force-pushed the coco_reader_rework2 branch from 0fac87e to aeb459f Compare October 29, 2020 15:17

Fixes

2917013

Signed-off-by: Joaquin Anton <janton@nvidia.com>

jantonguirao force-pushed the coco_reader_rework2 branch from 82cf81a to 2917013 Compare October 29, 2020 19:20

jantonguirao merged commit 523d56d into NVIDIA:master Oct 30, 2020

	DALI_ENFORCE(file.good(), make_string("Error writing to path: ", path));
	DALI_ENFORCE(file.good(), make_string("CocoReader meta file error while loading for path: ", path));

	DALI_ENFORCE(file, "CocoReader meta file error while loading for path: " + path);
	DALI_ENFORCE(file.good(), make_string("CocoReader meta file error while loading for path: ", path));

	auto segm_meta = annotation.poly_.segm_meta_.data();
	auto &segm_meta = annotation.poly_.segm_meta_;

Improvements in COCO reader API #2406

Improvements in COCO reader API #2406

Conversation

jantonguirao commented Oct 27, 2020 • edited Loading

Why we need this PR?

What happened in this PR?

jantonguirao commented Oct 27, 2020

dali-automaton commented Oct 27, 2020

dali-automaton commented Oct 27, 2020

Choose a reason for hiding this comment

review-notebook-app bot commented Oct 28, 2020 • edited Loading

jantonguirao commented Oct 28, 2020

JanuszL commented on 2020-10-28T11:37:02Z

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

jantonguirao commented Oct 28, 2020

dali-automaton commented Oct 28, 2020

dali-automaton commented Oct 28, 2020

jantonguirao commented Oct 28, 2020

dali-automaton commented Oct 28, 2020

dali-automaton commented Oct 28, 2020

jantonguirao commented Oct 29, 2020

dali-automaton commented Oct 29, 2020

dali-automaton commented Oct 29, 2020

Choose a reason for hiding this comment

jantonguirao commented Oct 29, 2020

dali-automaton commented Oct 29, 2020

dali-automaton commented Oct 29, 2020

jantonguirao commented Oct 29, 2020

dali-automaton commented Oct 29, 2020

jantonguirao commented Oct 29, 2020

dali-automaton commented Oct 29, 2020

dali-automaton commented Oct 29, 2020

jantonguirao commented Oct 29, 2020

dali-automaton commented Oct 29, 2020

dali-automaton commented Oct 30, 2020

jantonguirao commented Oct 27, 2020 •

edited

Loading

review-notebook-app bot commented Oct 28, 2020 •

edited

Loading