diff --git a/changelog.d/20241004_144626_mzhiltso_fix_task_creation_with_gt_job.md b/changelog.d/20241004_144626_mzhiltso_fix_task_creation_with_gt_job.md new file mode 100644 index 00000000000..c5a10acd207 --- /dev/null +++ b/changelog.d/20241004_144626_mzhiltso_fix_task_creation_with_gt_job.md @@ -0,0 +1,6 @@ +### Fixed + +- Invalid chunks for GT jobs when `?number` is used in the request and task frame step > 1 + () +- Invalid output of frames for specific GT frame requests with `api/jobs/{id}/data/?type=frame` + () diff --git a/cvat/apps/engine/cache.py b/cvat/apps/engine/cache.py index e3ad9051b45..06bf1a2dac5 100644 --- a/cvat/apps/engine/cache.py +++ b/cvat/apps/engine/cache.py @@ -504,8 +504,7 @@ def get_frames(): frame_range = ( ( db_data.start_frame - + chunk_number * db_data.chunk_size - + chunk_frame_idx * frame_step + + (chunk_number * db_data.chunk_size + chunk_frame_idx) * frame_step ) for chunk_frame_idx in range(db_data.chunk_size) ) diff --git a/cvat/apps/engine/frame_provider.py b/cvat/apps/engine/frame_provider.py index baab1f640dd..b9c95362661 100644 --- a/cvat/apps/engine/frame_provider.py +++ b/cvat/apps/engine/frame_provider.py @@ -485,7 +485,7 @@ def __len__(self): return self._db_segment.frame_count def validate_frame_number(self, frame_number: int) -> Tuple[int, int, int]: - frame_sequence = list(self._db_segment.frame_set) + frame_sequence = sorted(self._db_segment.frame_set) abs_frame_number = self._get_abs_frame_number(self._db_segment.task.data, frame_number) if abs_frame_number not in frame_sequence: raise ValidationError(f"Incorrect requested frame number: {frame_number}") diff --git a/cvat/apps/engine/serializers.py b/cvat/apps/engine/serializers.py index 0b440a157fc..3ba2a87da77 100644 --- a/cvat/apps/engine/serializers.py +++ b/cvat/apps/engine/serializers.py @@ -822,6 +822,7 @@ def create(self, validated_data): "The number of validation frames is not specified" ) + task_frame_provider = TaskFrameProvider(task) seed = validated_data.pop("random_seed", None) # The RNG backend must not change to yield reproducible results, @@ -832,7 +833,7 @@ def create(self, validated_data): frames: list[int] = [] overlap = task.overlap for segment in task.segment_set.all(): - segment_frames = set(segment.frame_set) + segment_frames = set(map(task_frame_provider.get_rel_frame_number, segment.frame_set)) selected_frames = segment_frames.intersection(frames) selected_count = len(selected_frames) @@ -841,12 +842,14 @@ def create(self, validated_data): continue selectable_segment_frames = set( - sorted(segment.frame_set)[overlap * (segment.start_frame != 0) : ] + sorted(segment_frames)[overlap * (segment.start_frame != 0) : ] ).difference(selected_frames) frames.extend(rng.choice( tuple(selectable_segment_frames), size=missing_count, replace=False ).tolist()) + + frames = list(map(task_frame_provider.get_abs_frame_number, frames)) elif frame_selection_method == models.JobFrameSelectionMethod.MANUAL: frames = validated_data.pop("frames") diff --git a/cvat/apps/engine/task.py b/cvat/apps/engine/task.py index b4ad868e197..459a6ddd4b3 100644 --- a/cvat/apps/engine/task.py +++ b/cvat/apps/engine/task.py @@ -1398,6 +1398,9 @@ def _update_status(msg: str) -> None: seed = validation_params.get("random_seed") rng = random.Generator(random.MT19937(seed=seed)) + def _to_rel_frame(abs_frame: int) -> int: + return (abs_frame - db_data.start_frame) // db_data.get_frame_step() + match validation_params["frame_selection_method"]: case models.JobFrameSelectionMethod.RANDOM_UNIFORM: all_frames = range(db_data.size) @@ -1431,7 +1434,7 @@ def _update_status(msg: str) -> None: validation_frames: list[int] = [] overlap = db_task.overlap for segment in db_task.segment_set.all(): - segment_frames = set(segment.frame_set) + segment_frames = set(map(_to_rel_frame, segment.frame_set)) selected_frames = segment_frames.intersection(validation_frames) selected_count = len(selected_frames) @@ -1440,7 +1443,7 @@ def _update_status(msg: str) -> None: continue selectable_segment_frames = set( - sorted(segment.frame_set)[overlap * (segment.start_frame != 0) : ] + sorted(segment_frames)[overlap * (segment.start_frame != 0) : ] ).difference(selected_frames) validation_frames.extend(rng.choice( @@ -1457,7 +1460,7 @@ def _update_status(msg: str) -> None: ) validation_frames: list[int] = [] - known_frame_names = {frame.path: frame.frame for frame in images} + known_frame_names = {frame.path: _to_rel_frame(frame.frame) for frame in images} unknown_requested_frames = [] for frame_filename in validation_params['frames']: frame_id = known_frame_names.get(frame_filename) @@ -1484,11 +1487,14 @@ def _update_status(msg: str) -> None: # TODO: refactor if hasattr(db_data, 'validation_layout'): if db_data.validation_layout.mode == models.ValidationMode.GT: + def _to_abs_frame(rel_frame: int) -> int: + return rel_frame * db_data.get_frame_step() + db_data.start_frame + db_gt_segment = models.Segment( task=db_task, start_frame=0, stop_frame=db_data.size - 1, - frames=db_data.validation_layout.frames, + frames=list(map(_to_abs_frame, db_data.validation_layout.frames)), type=models.SegmentType.SPECIFIC_FRAMES, ) elif db_data.validation_layout.mode == models.ValidationMode.GT_POOL: @@ -1653,7 +1659,7 @@ def _get_frame_size(frame_tuple: Tuple[av.VideoFrame, Any, Any]) -> int: (abs_frame_id - media_extractor.start) // media_extractor.step for abs_frame_id in ( frame_map.get(frame, frame) - for frame in db_segment.frame_set + for frame in sorted(db_segment.frame_set) ) ), lambda _: next(frame_counter) // db_data.chunk_size diff --git a/tests/python/rest_api/test_analytics.py b/tests/python/rest_api/test_analytics.py index 62a4bb8e144..7ac3004e63f 100644 --- a/tests/python/rest_api/test_analytics.py +++ b/tests/python/rest_api/test_analytics.py @@ -59,7 +59,7 @@ def _create_project(user, spec, **kwargs): return project.id, response.headers.get("X-Request-Id") @pytest.fixture(autouse=True) - def setup(self, restore_clickhouse_db_per_function): + def setup(self, restore_clickhouse_db_per_function, restore_redis_inmem_per_function): project_spec = { "name": f"Test project created by {self._USERNAME}", "labels": [ diff --git a/tests/python/rest_api/test_tasks.py b/tests/python/rest_api/test_tasks.py index 903c938561d..0a9ee61a124 100644 --- a/tests/python/rest_api/test_tasks.py +++ b/tests/python/rest_api/test_tasks.py @@ -2383,6 +2383,16 @@ def test_can_create_task_with_gt_job_from_images( assert len(gt_job_metas) == 1 + if frame_selection_method in ("random_uniform", "manual"): + assert gt_job_metas[0].size == validation_frames_count + elif frame_selection_method == "random_per_job": + assert gt_job_metas[0].size == ( + resulting_task_size // segment_size * validation_per_job_count + + min(resulting_task_size % segment_size, validation_per_job_count) + ) + else: + assert False + assert task.segment_size == segment_size assert task.size == resulting_task_size assert task_meta.size == resulting_task_size @@ -2494,6 +2504,16 @@ def test_can_create_task_with_gt_job_from_video( assert len(gt_job_metas) == 1 + if frame_selection_method == "random_uniform": + assert gt_job_metas[0].size == validation_frames_count + elif frame_selection_method == "random_per_job": + assert gt_job_metas[0].size == ( + resulting_task_size // segment_size * validation_per_job_count + + min(resulting_task_size % segment_size, validation_per_job_count) + ) + else: + assert False + assert task.segment_size == segment_size assert task.size == resulting_task_size assert task_meta.size == resulting_task_size @@ -2621,10 +2641,16 @@ def _uploaded_images_task_fxt_base( frame_count = len(image_files) images_data = [f.getvalue() for f in image_files] + + resulting_task_size = len( + range(start_frame or 0, (stop_frame or len(images_data) - 1) + 1, step or 1) + ) + data_params = { "image_quality": 70, "client_files": image_files, "sorting_method": "natural", + "chunk_size": max(1, (segment_size or resulting_task_size) // 2), } data_params.update(data_kwargs) @@ -2645,7 +2671,7 @@ def get_frame(i: int) -> bytes: models.TaskWriteRequest._from_openapi_data(**task_params), models.DataRequest._from_openapi_data(**data_params), get_frame=get_frame, - size=len(range(start_frame or 0, (stop_frame or len(images_data) - 1) + 1, step or 1)), + size=resulting_task_size, ), task_id @pytest.fixture(scope="class") @@ -2756,6 +2782,81 @@ def fxt_uploaded_images_task_with_honeypots_and_segments_start_step( request, start_frame=start_frame, step=step ) + def _uploaded_images_task_with_gt_and_segments_base( + self, + request: pytest.FixtureRequest, + *, + start_frame: Optional[int] = None, + step: Optional[int] = None, + frame_selection_method: str = "random_uniform", + ) -> Generator[Tuple[_TaskSpec, int], None, None]: + used_frames_count = 16 + total_frame_count = (start_frame or 0) + used_frames_count * (step or 1) + segment_size = 5 + image_files = generate_image_files(total_frame_count) + + validation_params_kwargs = {"frame_selection_method": frame_selection_method} + + if "random" in frame_selection_method: + validation_params_kwargs["random_seed"] = 42 + + if frame_selection_method == "random_uniform": + validation_frames_count = 10 + validation_params_kwargs["frame_count"] = validation_frames_count + elif frame_selection_method == "random_per_job": + frames_per_job_count = 3 + validation_params_kwargs["frames_per_job_count"] = frames_per_job_count + validation_frames_count = used_frames_count // segment_size + min( + used_frames_count % segment_size, frames_per_job_count + ) + elif frame_selection_method == "manual": + validation_frames_count = 10 + + valid_frame_ids = range( + (start_frame or 0), (start_frame or 0) + used_frames_count * (step or 1), step or 1 + ) + rng = np.random.Generator(np.random.MT19937(seed=42)) + validation_params_kwargs["frames"] = rng.choice( + [f.name for i, f in enumerate(image_files) if i in valid_frame_ids], + validation_frames_count, + replace=False, + ).tolist() + else: + raise NotImplementedError + + validation_params = models.DataRequestValidationParams._from_openapi_data( + mode="gt", + **validation_params_kwargs, + ) + + yield from self._uploaded_images_task_fxt_base( + request=request, + frame_count=None, + image_files=image_files, + segment_size=segment_size, + sorting_method="natural", + start_frame=start_frame, + step=step, + validation_params=validation_params, + ) + + @fixture(scope="class") + @parametrize("start_frame, step", [(2, 3)]) + @parametrize("frame_selection_method", ["random_uniform", "random_per_job", "manual"]) + def fxt_uploaded_images_task_with_gt_and_segments_start_step( + self, + request: pytest.FixtureRequest, + start_frame: Optional[int], + step: Optional[int], + frame_selection_method: str, + ) -> Generator[Tuple[_TaskSpec, int], None, None]: + yield from self._uploaded_images_task_with_gt_and_segments_base( + request, + start_frame=start_frame, + step=step, + frame_selection_method=frame_selection_method, + ) + def _uploaded_video_task_fxt_base( self, request: pytest.FixtureRequest, @@ -2773,11 +2874,16 @@ def _uploaded_video_task_fxt_base( if segment_size: task_params["segment_size"] = segment_size + resulting_task_size = len( + range(start_frame or 0, (stop_frame or frame_count - 1) + 1, step or 1) + ) + video_file = generate_video_file(frame_count) video_data = video_file.getvalue() data_params = { "image_quality": 70, "client_files": [video_file], + "chunk_size": max(1, (segment_size or resulting_task_size) // 2), } if start_frame is not None: @@ -2797,7 +2903,7 @@ def get_video_file() -> io.BytesIO: models.TaskWriteRequest._from_openapi_data(**task_params), models.DataRequest._from_openapi_data(**data_params), get_video_file=get_video_file, - size=len(range(start_frame or 0, (stop_frame or frame_count - 1) + 1, step or 1)), + size=resulting_task_size, ), task_id @pytest.fixture(scope="class") @@ -2834,10 +2940,10 @@ def _compute_annotation_segment_params(self, task_spec: _TaskSpec) -> List[Tuple frame_step = task_spec.frame_step segment_size = getattr(task_spec, "segment_size", 0) or task_spec.size * frame_step start_frame = getattr(task_spec, "start_frame", 0) - end_frame = ( - getattr(task_spec, "stop_frame", None) or ((task_spec.size - 1) * frame_step) - ) + frame_step - end_frame = end_frame - ((end_frame - frame_step - start_frame) % frame_step) + stop_frame = getattr(task_spec, "stop_frame", None) or ( + start_frame + (task_spec.size - 1) * frame_step + ) + end_frame = stop_frame - ((stop_frame - start_frame) % frame_step) + frame_step validation_params = getattr(task_spec, "validation_params", None) if validation_params and validation_params.mode.value == "gt_pool": @@ -2881,22 +2987,40 @@ def _compare_images( else: assert np.array_equal(chunk_frame_pixels, expected_pixels) + def _get_job_abs_frame_set(self, job_meta: models.DataMetaRead) -> Sequence[int]: + if job_meta.included_frames: + return job_meta.included_frames + else: + return range( + job_meta.start_frame, + job_meta.stop_frame + 1, + parse_frame_step(job_meta.frame_filter), + ) + _tasks_with_honeypots_cases = [ fixture_ref("fxt_uploaded_images_task_with_honeypots_and_segments"), fixture_ref("fxt_uploaded_images_task_with_honeypots_and_segments_start_step"), ] + _tasks_with_simple_gt_job_cases = [ + fixture_ref("fxt_uploaded_images_task_with_gt_and_segments_start_step") + ] + # Keep in mind that these fixtures are generated eagerly # (before each depending test or group of tests), # e.g. a failing task creation in one the fixtures will fail all the depending tests cases. - _all_task_cases = [ - fixture_ref("fxt_uploaded_images_task"), - fixture_ref("fxt_uploaded_images_task_with_segments"), - fixture_ref("fxt_uploaded_images_task_with_segments_start_stop_step"), - fixture_ref("fxt_uploaded_video_task"), - fixture_ref("fxt_uploaded_video_task_with_segments"), - fixture_ref("fxt_uploaded_video_task_with_segments_start_stop_step"), - ] + _tasks_with_honeypots_cases + _all_task_cases = ( + [ + fixture_ref("fxt_uploaded_images_task"), + fixture_ref("fxt_uploaded_images_task_with_segments"), + fixture_ref("fxt_uploaded_images_task_with_segments_start_stop_step"), + fixture_ref("fxt_uploaded_video_task"), + fixture_ref("fxt_uploaded_video_task_with_segments"), + fixture_ref("fxt_uploaded_video_task_with_segments_start_stop_step"), + ] + + _tasks_with_honeypots_cases + + _tasks_with_simple_gt_job_cases + ) @parametrize("task_spec, task_id", _all_task_cases) def test_can_get_task_meta(self, task_spec: _TaskSpec, task_id: int): @@ -2980,13 +3104,17 @@ def test_can_get_task_chunks(self, task_spec: _TaskSpec, task_id: int): else: assert False - task_frames = range( + task_abs_frames = range( task_meta.start_frame, task_meta.stop_frame + 1, task_spec.frame_step ) task_chunk_frames = [ (chunk_number, list(chunk_frames)) for chunk_number, chunk_frames in groupby( - task_frames, key=lambda frame: frame // task_meta.chunk_size + task_abs_frames, + key=lambda abs_frame: ( + (abs_frame - task_meta.start_frame) // task_spec.frame_step + ) + // task_meta.chunk_size, ) ] for quality, (chunk_id, expected_chunk_frame_ids) in product( @@ -3022,7 +3150,7 @@ def test_can_get_task_chunks(self, task_spec: _TaskSpec, task_id: int): ) @parametrize("task_spec, task_id", _all_task_cases) - def test_can_get_job_meta(self, task_spec: _TaskSpec, task_id: int): + def test_can_get_annotation_job_meta(self, task_spec: _TaskSpec, task_id: int): segment_params = self._compute_annotation_segment_params(task_spec) with make_api_client(self._USERNAME) as api_client: @@ -3043,10 +3171,15 @@ def test_can_get_job_meta(self, task_spec: _TaskSpec, task_id: int): segment_size = math.ceil((segment_stop - segment_start + 1) / task_spec.frame_step) assert job_meta.size == segment_size - job_frame_set = set( - range(job_meta.start_frame, job_meta.stop_frame + 1, task_spec.frame_step) + job_abs_frame_set = self._get_job_abs_frame_set(job_meta) + assert len(job_abs_frame_set) == job_meta.size + assert set(job_abs_frame_set).issubset( + range( + job_meta.start_frame, + job_meta.stop_frame + 1, + parse_frame_step(job_meta.frame_filter), + ) ) - assert len(job_frame_set) == job_meta.size if getattr(task_spec, "chunk_size", None): assert job_meta.chunk_size == task_spec.chunk_size @@ -3056,6 +3189,62 @@ def test_can_get_job_meta(self, task_spec: _TaskSpec, task_id: int): else: assert len(job_meta.frames) == job_meta.size + @parametrize("task_spec, task_id", _tasks_with_simple_gt_job_cases) + def test_can_get_simple_gt_job_meta(self, task_spec: _TaskSpec, task_id: int): + with make_api_client(self._USERNAME) as api_client: + jobs = sorted( + get_paginated_collection( + api_client.jobs_api.list_endpoint, task_id=task_id, type="ground_truth" + ), + key=lambda j: j.start_frame, + ) + assert len(jobs) == 1 + + gt_job = jobs[0] + (job_meta, _) = api_client.jobs_api.retrieve_data_meta(gt_job.id) + + task_start_frame = getattr(task_spec, "start_frame", 0) + assert (job_meta.start_frame, job_meta.stop_frame) == ( + task_start_frame, + task_start_frame + (task_spec.size - 1) * task_spec.frame_step, + ) + assert job_meta.frame_filter == getattr(task_spec, "frame_filter", "") + + frame_selection_method = task_spec.validation_params.frame_selection_method.value + if frame_selection_method == "random_uniform": + validation_frames_count = task_spec.validation_params.frame_count + elif frame_selection_method == "random_per_job": + frames_per_job_count = task_spec.validation_params.frames_per_job_count + validation_frames_count = ( + task_spec.size // task_spec.segment_size * frames_per_job_count + + min(task_spec.size % task_spec.segment_size, frames_per_job_count) + ) + elif frame_selection_method == "manual": + validation_frames_count = len(task_spec.validation_params.frames) + else: + raise NotImplementedError(frame_selection_method) + + assert job_meta.size == validation_frames_count + + job_abs_frame_set = self._get_job_abs_frame_set(job_meta) + assert len(job_abs_frame_set) == job_meta.size + assert set(job_abs_frame_set).issubset( + range( + job_meta.start_frame, + job_meta.stop_frame + 1, + parse_frame_step(job_meta.frame_filter), + ) + ) + + if getattr(task_spec, "chunk_size", None): + assert job_meta.chunk_size == task_spec.chunk_size + + if task_spec.source_data_type == _SourceDataType.video: + assert len(job_meta.frames) == 1 + else: + # there are placeholders on the non-included places + assert len(job_meta.frames) == task_spec.size + @parametrize("task_spec, task_id", _tasks_with_honeypots_cases) def test_can_get_honeypot_gt_job_meta(self, task_spec: _TaskSpec, task_id: int): with make_api_client(self._USERNAME) as api_client: @@ -3098,12 +3287,11 @@ def test_can_get_job_frames(self, task_spec: _TaskSpec, task_id: int): ) for job in jobs: (job_meta, _) = api_client.jobs_api.retrieve_data_meta(job.id) + job_abs_frames = self._get_job_abs_frame_set(job_meta) for quality, (frame_pos, abs_frame_id) in product( ["original", "compressed"], - enumerate( - range(job_meta.start_frame, job_meta.stop_frame, task_spec.frame_step) - ), + enumerate(job_abs_frames), ): rel_frame_id = ( abs_frame_id - getattr(task_spec, "start_frame", 0) @@ -3139,6 +3327,8 @@ def test_can_get_job_frames(self, task_spec: _TaskSpec, task_id: int): @parametrize("task_spec, task_id", _all_task_cases) @parametrize("indexing", ["absolute", "relative"]) def test_can_get_job_chunks(self, task_spec: _TaskSpec, task_id: int, indexing: str): + _placeholder_image = Image.fromarray(np.zeros((1, 1, 3), dtype=np.uint8)) + with make_api_client(self._USERNAME) as api_client: jobs = sorted( get_paginated_collection(api_client.jobs_api.list_endpoint, task_id=task_id), @@ -3150,6 +3340,9 @@ def test_can_get_job_chunks(self, task_spec: _TaskSpec, task_id: int, indexing: for job in jobs: (job_meta, _) = api_client.jobs_api.retrieve_data_meta(job.id) + if job_meta.included_frames: + assert len(job_meta.included_frames) == job_meta.size + if task_spec.source_data_type == _SourceDataType.images: assert job.data_original_chunk_type == "imageset" assert job.data_compressed_chunk_type == "imageset" @@ -3164,7 +3357,7 @@ def test_can_get_job_chunks(self, task_spec: _TaskSpec, task_id: int, indexing: assert False if indexing == "absolute": - chunk_count = math.ceil(task_meta.size / job_meta.chunk_size) + chunk_count = math.ceil(task_meta.size / task_meta.chunk_size) def get_task_chunk_abs_frame_ids(chunk_id: int) -> Sequence[int]: return range( @@ -3196,18 +3389,10 @@ def get_expected_chunk_abs_frame_ids(chunk_id: int): job_chunk_ids = range(chunk_count) def get_expected_chunk_abs_frame_ids(chunk_id: int): - return sorted( - frame - for frame in range( - job_meta.start_frame - + chunk_id * job_meta.chunk_size * task_spec.frame_step, - job_meta.start_frame - + min((chunk_id + 1) * job_meta.chunk_size, job_meta.size) - * task_spec.frame_step, - task_spec.frame_step, - ) - if not job_meta.included_frames or frame in job_meta.included_frames - ) + job_abs_frames = self._get_job_abs_frame_set(job_meta) + return job_abs_frames[ + chunk_id * job_meta.chunk_size : (chunk_id + 1) * job_meta.chunk_size + ] for quality, chunk_id in product(["original", "compressed"], job_chunk_ids): expected_chunk_abs_frame_ids = get_expected_chunk_abs_frame_ids(chunk_id) @@ -3241,13 +3426,24 @@ def get_expected_chunk_abs_frame_ids(chunk_id: int): else: chunk_images = dict(enumerate(read_video_file(chunk_file))) - assert sorted(chunk_images.keys()) == list(range(job_meta.size)) + assert sorted(chunk_images.keys()) == list( + range(len(expected_chunk_abs_frame_ids)) + ) for chunk_frame, abs_frame_id in zip( chunk_images, expected_chunk_abs_frame_ids ): + if ( + indexing == "absolute" + and job_meta.included_frames + and abs_frame_id not in job_meta.included_frames + ): + expected_image = _placeholder_image + else: + expected_image = task_spec.read_frame(abs_frame_id) + self._compare_images( - task_spec.read_frame(abs_frame_id), + expected_image, chunk_images[chunk_frame], must_be_identical=( task_spec.source_data_type == _SourceDataType.images @@ -3509,6 +3705,7 @@ def test_work_with_task_containing_non_stable_cloud_storage_files( @pytest.mark.usefixtures("restore_redis_inmem_per_function") +@pytest.mark.usefixtures("restore_redis_ondisk_per_class") class TestTaskBackups: @pytest.fixture(autouse=True) def setup( @@ -4110,6 +4307,7 @@ def test_task_unassigned_cannot_see_task_preview( self._test_assigned_users_cannot_see_task_preview(tasks, users, is_task_staff) +@pytest.mark.usefixtures("restore_redis_ondisk_per_class") class TestUnequalJobs: @pytest.fixture(autouse=True) def setup(self, restore_db_per_function, tmp_path: Path, admin_user: str):