Add a way to create task with custom jobs #5536

zhiltsov-max · 2022-12-30T12:38:07Z

Motivation and context

This PR adds an option to specify file to job mapping explicitly during task creation. This option is incompatible with most other job-related parameters like sorting_method and frame_step.

Added a new task creation parameter (job_file_mapping) to set a custom file to job mapping during task creation

How has this been tested?

Unit tests, manually

Checklist

I submit my changes into the develop branch
I have added a description of my changes into CHANGELOG file
I have updated the documentation accordingly
I have added tests to cover my changes
I have linked related issues (read github docs)
I have increased versions of npm packages if it is necessary (cvat-canvas,
cvat-core, cvat-data and cvat-ui)

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.

nmanovic · 2023-01-05T09:20:48Z

cvat/apps/engine/media_extractors.py

@@ -153,7 +153,7 @@ def __init__(self,
        if not source_path:
            raise Exception('No image found')

-        if stop is None:
+        if not stop:


Why have you changed the condition? What is the reason to touch the code?

When we import backup, there is the call to reconcile, which chains to the c-tor. The stop_frame is set to 0 after the default value of the DataSerializer here
In the reconcile args, stop_frame=0 makes little sense on its own, but it produces a dataset with only 1 frame.

The possible fixes were:

clean input values to the _create_thread function when importing backups

replace the input value with None or remove it when importing backups (which contradicts the serializer)

change the default value in the serializer (which refers to the model)

I've decided to make it behave as None, because 0 is the default value.

cvat/apps/engine/serializers.py

SpecLad · 2023-01-05T14:32:43Z

cvat/apps/engine/serializers.py

+class JobFiles(serializers.ListField):
+    """
+    Read JobFileMapping docs for more info.
+    """
+
+    def __init__(self, *args, **kwargs):
+        kwargs.setdefault('child', serializers.CharField(allow_blank=False, max_length=1024))
+        kwargs.setdefault('allow_empty', False)
+        super().__init__(*args, **kwargs)


Does this need to be a distinct class? Seems like it could be reduced to serializers.ListField(child=..., allow_empty=False) and inlined.

Yes, it can be inlined. The class is introduced to encapsulate the logic behind parameters. I'd do it to most of the other complex fields too, but it's quite a big change.

The class is introduced to encapsulate the logic behind parameters.

What do you mean?

There is no need for using classes to know the internal structure or construction details of this class.

cvat/apps/engine/task.py

SpecLad · 2023-01-05T15:48:37Z

cvat/apps/engine/task.py

+        if start_frame < 0:
+            raise ValidationError(
+                f"Failed to create segment: invalid start frame {start_frame}"
+            )

-        slogger.glob.info("New segment for task #{}: start_frame = {}, \
-            stop_frame = {}".format(db_task.id, start_frame, stop_frame))
+        if stop_frame >= db_task.data.size:
+            raise ValidationError(
+                f"Failed to create segment: stop frame {stop_frame} is beyond task size"
+            )


How can either of these be possible? Shouldn't _get_task_segment_data always return valid segments?

Yes, it should, but it's not a question for this function. Hopefully, they should never fire.

If they're never supposed to fire, shouldn't they be asserts?

Yes, they can be asserts, currently. If new job creation methods appear, the checks may become more actual.

Upd: I've decided to remove the checks, because these conditions are checked in the tests and they are required mostly for development.

cvat/apps/engine/task.py

SpecLad · 2023-01-05T16:03:10Z

cvat/apps/engine/models.py

@@ -205,6 +205,12 @@ class Data(models.Model):
    sorting_method = models.CharField(max_length=15, choices=SortingMethod.choices(), default=SortingMethod.LEXICOGRAPHICAL)
    deleted_frames = IntArrayField(store_sorted=True, unique_values=True)

+    # Avoid storing whole mapping here, its redundant
+    custom_segments = models.BooleanField(default=False)


Can't this be determined by checking if segment_size is 0 on the task? Maybe we don't need this new field?

I also thought that this variable can be excluded. Basically, we'd need to check the segment size and the number of jobs != 1. However, it turned out that in the case of 1 job in the task, the cases are hard to distinguish. I've decided then to add a clear indicator instead.

Is it possible to have non-zero segment_size when there's no custom mapping? I thought segment_size defaulted to the number of frames.

Is it possible to have non-zero segment_size when there's no custom mapping?

Yes, it represents limitations on the size of a single job. The comment says about this field:

Zero means that there are no limits (default)

nmanovic · 2023-01-05T16:53:11Z

@zhiltsov-max , #4869 (related issue)

SpecLad · 2023-01-06T16:30:47Z

cvat/apps/engine/backup.py

+            if self._db_task.mode == 'annotation':
+                files: Iterable[models.Image] = self._db_data.images.all().order_by('frame')
+                segment_files = files[db_segment.start_frame : db_segment.stop_frame + 1]
+                return {'files': list(frame.path for frame in segment_files)}


Isn't this redundant information? The backups already have a manifest, which records the frames in order; given that and the segment boundaries, you should be able to reconstruct the list of files in each segment.

Backups have such manifest only in some cases. Also, it will be simpler to extend this parameter in future if it is saved separately. For instance, if jobs can contain arbitrary files or have overlaps.

nmanovic · 2023-01-12T07:46:20Z

@zhiltsov-max , could you please resolve conflicts?

nmanovic · 2023-01-12T12:31:17Z

cvat/apps/engine/task.py

@@ -387,11 +474,17 @@ def _create_thread(db_task, data, isBackupRestore=False, isDatasetImport=False):
    media = _count_files(data)
    media, task_mode = _validate_data(media, manifest_files)

+    if job_file_mapping is not None and task_mode != 'annotation':


Should it be a part of _validate_job_file_mapping?

The task mode is unknown prior to the previous line. The _validate_job_file_mapping call can be moved later, but it looks like it's better to fail as soon as possible in case of invalid parameters.

nmanovic · 2023-01-12T12:33:09Z

cvat/apps/engine/task.py

-        manifest = ImageManifestManager(db_data.get_manifest_path())
-        manifest.set_index()
+    # Sort the files
+    if (isBackupRestore and (


Probably it is time to move the code into a separate function?

My suggestion is to do it in another PR, because it will simplify life in case of merge conflicts and failing tests.

nmanovic · 2023-01-12T13:14:43Z

Our conclusion after the internal discussion:

Need to remove custom_segments field. (it should be addressed in the PR)
Basically, we have a configuration for a task (how to split it on multiple jobs). It can be defined in multiple ways. Right now we store in DB extra information as segment_size, sorting_method, etc. Indeed we should create segments and jobs, put images in the right order into the table.

This PR adds an option to specify file to job mapping explicitly during task creation. This option is incompatible with most other job-related parameters like `sorting_method` and `frame_step`. - Added a new task creation parameter (`job_file_mapping`) to set a custom file to job mapping during task creation

Add a way to create task with custom jobs

bca4d11

zhiltsov-max requested review from azhavoro and nmanovic as code owners December 30, 2022 12:38

zhiltsov-max added 3 commits December 30, 2022 15:39

Merge branch 'develop' into zm/custom-jobs

9428bd0

Revert debug changes

3f5c510

Merge remote-tracking branch 'origin/zm/custom-jobs' into zm/custom-jobs

ae26534

zhiltsov-max changed the title ~~[WIP] Add a way to create task with custom jobs~~ [do not merge] Add a way to create task with custom jobs Dec 30, 2022

zhiltsov-max added 2 commits December 30, 2022 16:57

Fix backups

054d72b

Fix type annotation

e5f004d

nmanovic changed the title ~~[do not merge] Add a way to create task with custom jobs~~ [WIP] Add a way to create task with custom jobs Dec 31, 2022

zhiltsov-max added 3 commits January 3, 2023 15:01

Support custom job sizes in backups

421ff0f

Update tests

a2beeb1

Update changelog

79b6c3d

zhiltsov-max changed the title ~~[WIP] Add a way to create task with custom jobs~~ Add a way to create task with custom jobs Jan 3, 2023

zhiltsov-max added 2 commits January 3, 2023 15:10

Merge branch 'develop' into zm/custom-jobs

f9eadbb

Fix missing variable

2ec8bdf

nmanovic mentioned this pull request Jan 4, 2023

Cannot recreate project/task from backup with sorting method predefined and no cache #5042

Open

1 task

nmanovic added the server label Jan 5, 2023

nmanovic requested review from SpecLad and removed request for azhavoro January 5, 2023 09:15

nmanovic reviewed Jan 5, 2023

View reviewed changes

SpecLad reviewed Jan 5, 2023

View reviewed changes

nmanovic mentioned this pull request Jan 5, 2023

Split dataset after task creation #4869

Closed

zhiltsov-max added 4 commits January 6, 2023 11:15

Update error message

d7b3ae1

Fix review comments

6d336c6

Fix imports

2db847e

Remove extra validation checks

4747148

SpecLad reviewed Jan 6, 2023

View reviewed changes

zhiltsov-max and others added 3 commits January 12, 2023 11:02

Merge branch 'develop' into zm/custom-jobs

db32950

Fix merge

d0009a0

Merge branch 'develop' into zm/custom-jobs

a6fd10d

nmanovic reviewed Jan 12, 2023

View reviewed changes

nmanovic and others added 3 commits January 12, 2023 21:17

Merge branch 'develop' into zm/custom-jobs

6cb957f

Remove extra field in db

7135332

Merge remote-tracking branch 'origin/zm/custom-jobs' into zm/custom-jobs

556a4fc

nmanovic approved these changes Jan 13, 2023

View reviewed changes

nmanovic merged commit 31f0578 into develop Jan 13, 2023

nmanovic deleted the zm/custom-jobs branch January 13, 2023 16:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a way to create task with custom jobs #5536

Add a way to create task with custom jobs #5536

zhiltsov-max commented Dec 30, 2022 •

edited

Loading

nmanovic Jan 5, 2023

zhiltsov-max Jan 5, 2023

SpecLad Jan 5, 2023

zhiltsov-max Jan 5, 2023

SpecLad Jan 6, 2023

zhiltsov-max Jan 6, 2023

SpecLad Jan 5, 2023

zhiltsov-max Jan 5, 2023

SpecLad Jan 6, 2023

zhiltsov-max Jan 6, 2023 •

edited

Loading

SpecLad Jan 5, 2023

zhiltsov-max Jan 5, 2023

SpecLad Jan 6, 2023

zhiltsov-max Jan 6, 2023

nmanovic commented Jan 5, 2023

SpecLad Jan 6, 2023

zhiltsov-max Jan 6, 2023

nmanovic commented Jan 12, 2023

nmanovic Jan 12, 2023

zhiltsov-max Jan 13, 2023

nmanovic Jan 12, 2023

zhiltsov-max Jan 13, 2023

nmanovic commented Jan 12, 2023

Add a way to create task with custom jobs #5536

Add a way to create task with custom jobs #5536

Conversation

zhiltsov-max commented Dec 30, 2022 • edited Loading

Motivation and context

How has this been tested?

Checklist

License

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

zhiltsov-max Jan 6, 2023 • 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

nmanovic commented Jan 5, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nmanovic commented Jan 12, 2023

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

nmanovic commented Jan 12, 2023

zhiltsov-max commented Dec 30, 2022 •

edited

Loading

zhiltsov-max Jan 6, 2023 •

edited

Loading