Add NDR by byeongkyu-kang · Pull Request #113 · open-edge-platform/datumaro

byeongkyu-kang · 2021-02-19T11:33:16Z

Summary

Added NDR to remove duplicated images in a dataset.

How to test

python -m unittest -v tests/test_ndr.py

Checklist

I submit my changes into the develop branch
I have added description of my changes into CHANGELOG
I have updated the documentation accordingly
I have added tests to cover my changes
I have linked related issues)

License

I submit my code changes under the same MIT License that covers the project.
Feel free to contact the maintainers if that's a concern.
I have updated the license header for each file (see an example below)

# Copyright (C) 2020 Intel Corporation
#
# SPDX-License-Identifier: MIT

zhiltsov-max

Hi, thanks for your contribution!
Please fix formatting - avoid lines longer than 80 characters.

zhiltsov-max · 2021-02-25T09:51:52Z

CHANGELOG.md

 - `Icdar13/15` dataset format (<https://github.com/openvinotoolkit/datumaro/pull/96>)
 - Laziness, source caching, tracking of changes and partial updating for `Dataset` (<https://github.com/openvinotoolkit/datumaro/pull/102>)
 - `Market-1501` dataset format (<https://github.com/openvinotoolkit/datumaro/pull/108>)
+- Add NDR (<https://github.com/openvinotoolkit/datumaro/pull/113>)


Please use the full form here instead of abbreviation to make it more clear for readers.

Okay, I will update.

zhiltsov-max · 2021-02-25T09:57:53Z

datumaro/plugins/ndr.py

+    def __init__(self, extractor, working_subset=None, duplicated_subset="duplicated", algorithm='gradient',
+            num_cut=None, over_sample='random', under_sample='uniform', seed=None, **kwargs):
+        """
+        Near-duplicated image removal


Hi, please include a couple of examples in the class doc and maybe some explanation for the CLI parameters, if this function is supposed to be used from CLI. An example can be found here: https://github.com/openvinotoolkit/datumaro/blob/develop/datumaro/plugins/transforms.py#L313

I planned to support CLI by inheriting CliPlugin in another PR. Do I need to write a documentation anyway?

Let's add it in another PR then.

zhiltsov-max · 2021-02-25T10:01:58Z

datumaro/plugins/ndr.py

+
+        Arguments
+        ---------------
+        working_subset: str


I'm not sure if using a parameter is better than passing just a subset as an input. Also, I think, it looks more natural, when None run the algorithm for the whole dataset.

I think that might return unexpected results if user has a subset like val or test.

zhiltsov-max · 2021-02-25T10:04:58Z

datumaro/plugins/ndr.py

+            raise ValueError("Invalid under_sample")
+
+        self.duplicated_subset = duplicated_subset
+        self._remove(algorithm, working_subset, num_cut, over_sample, under_sample, **kwargs)


Try to make the transform lazy - it should only be applied when __iter__ is called, look at "splitter" transforms for example. You'll also need to move the initialization of RNG into __iter__. Consider __iter__ in the input dataset a heavy operation.

Okay, I will check "splitter" and change to lazy transformation

zhiltsov-max · 2021-02-25T10:07:01Z

datumaro/plugins/ndr.py

+        super().__init__(extractor)
+        if seed:
+            np.random.seed(seed)
+        if len(extractor) < 1:


I don't think this situation should be considered an error.

Okay, I will update

zhiltsov-max · 2021-02-25T10:15:40Z

datumaro/plugins/ndr.py

+        if num_cut and num_cut > len(all_imgs):
+            raise ValueError("The number of images is smaller than the cut you want")
+
+        if algorithm == "gradient":


Consider using an enum instead of a string constant.

Could you explain more about enum?

class Algo_name(Enum): algo_name1 = "..." .... ... if algorithm == Algo_name.algo_name1

Do you mean creating a class by inheriting Enum like above?

AlgoName = Enum('AlgoName', ['good', 'best', 'poor'])

Examples: https://github.com/openvinotoolkit/datumaro/blob/develop/datumaro/plugins/voc_format/format.py#L16, https://github.com/openvinotoolkit/datumaro/blob/develop/datumaro/cli/contexts/project/__init__.py#L228

zhiltsov-max · 2021-02-25T10:18:09Z

datumaro/plugins/ndr.py

+                continue
+
+            # Hash collision: compare dot-product based feature similarity
+            max_sim = np.max(np.dot(clr_dict[key], clr) ** 50)


Please make the use of 50 more clear. It is unclear if it is just a random number, or something with a strong mathematical proof.

We do not have any experimental results for that number. The number is for maximizing the gap between duplicated-images and non-duplicated-images and is set arbitrarily. Then do we need to change it as a parameter?

I don't have any complaints against the value, but I'd better make it a named constant to clarify its use.

Oh I understand. Okay, I will update

- Added support for direct subset access in datasets merged from several extractors if the format provides such support - Limited dataset reading to only the specified subset in streaming, when there are no transforms

byeongkyu-kang added 2 commits February 19, 2021 19:54

Initial commit for NDR

9aba5f9

Change CHANGELOG.md

62434b2

byeongkyu-kang requested a review from zhiltsov-max February 22, 2021 03:03

zhiltsov-max suggested changes Feb 25, 2021

View reviewed changes

byeongkyu-kang and others added 9 commits February 25, 2021 21:35

Update CHANGELOG.md clearly

0e0c9b6

Remove checking empty dataset code

5434a43

Change to lazy transformation and update tests

7641c55

Update code formatting

803d02b

Use Enum for algorithm names

6d585bd

Add some constants for readability

c23a987

Merge branch 'develop' into bk/initial-ndr

13d640a

style fixes

523838a

style fixes

e09ba69

zhiltsov-max merged commit 88d7df9 into develop Feb 26, 2021

zhiltsov-max deleted the bk/initial-ndr branch February 26, 2021 17:59

Conversation

byeongkyu-kang commented Feb 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

How to test

Checklist

License

Uh oh!

zhiltsov-max left a comment

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!

byeongkyu-kang Feb 25, 2021 • 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!

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!

byeongkyu-kang Feb 25, 2021 • 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

byeongkyu-kang commented Feb 19, 2021 •

edited

Loading

byeongkyu-kang Feb 25, 2021 •

edited

Loading

byeongkyu-kang Feb 25, 2021 •

edited

Loading