perf: consistenty and perf improvements (#170)

* Add validation-only mode to cvat_evaluation_pipeline and make output consistent

Signed-off-by: Christoph Auer <[email protected]>

* Revise CVAT XML parsing for efficiency

Signed-off-by: Christoph Auer <[email protected]>

---------

Signed-off-by: Christoph Auer <[email protected]>

Author: cau-git

docling_eval/campaign_tools/cvat_evaluation_pipeline.py

@@ -36,6 +36,14 @@
     find_xml_files_by_pattern,
     parse_cvat_folder,
 )
+from docling_eval.cvat_tools.models import (
+    CVATValidationError,
+    CVATValidationReport,
+    CVATValidationRunReport,
+    ValidationSeverity,
+)
+from docling_eval.cvat_tools.parser import get_all_images_from_cvat_xml, parse_cvat_file
+from docling_eval.cvat_tools.validator import Validator, validate_cvat_sample
 from docling_eval.datamodels.types import (
     BenchMarkNames,
     EvaluationModality,
@@ -139,8 +147,6 @@ def _convert_cvat_set_to_json(
         Returns:
             List of created JSON file paths
         """
-        from docling_eval.cvat_tools.models import CVATValidationRunReport
-
         folder_structure = self._load_folder_structure(xml_pattern)
 
         if output_json_dir.exists():
@@ -280,6 +286,78 @@ def merge_annotation_xmls(
 
         return gt_xml, pred_xml
 
+    def regenerate_validation_reports_from_merged(
+        self,
+        merged_dir: Optional[Path] = None,
+    ) -> None:
+        """Rebuild validation reports using pre-merged annotation XMLs."""
+
+        if merged_dir is None:
+            merged_dir = self.output_dir / "merged_xml"
+
+        set_to_filename = {
+            "set_A": merged_dir / "combined_set_A.xml",
+            "set_B": merged_dir / "combined_set_B.xml",
+        }
+
+        validator = Validator()
+
+        for set_label, xml_path in set_to_filename.items():
+            if not xml_path.exists():
+                raise FileNotFoundError(
+                    f"Missing merged annotations for {set_label}: {xml_path}"
+                )
+
+            parsed_file = parse_cvat_file(xml_path)
+            image_names = sorted(parsed_file.image_names)
+
+            reports: list[CVATValidationReport] = []
+            for image_name in image_names:
+                try:
+                    validated = validate_cvat_sample(
+                        xml_path,
+                        image_name,
+                        validator=validator,
+                        parsed_file=parsed_file,
+                    )
+                    reports.append(validated.report)
+                except Exception as exc:  # noqa: BLE001
+                    _log.error(
+                        "Validation failed for %s (%s): %s",
+                        set_label,
+                        image_name,
+                        exc,
+                    )
+                    reports.append(
+                        CVATValidationReport(
+                            sample_name=image_name,
+                            errors=[
+                                CVATValidationError(
+                                    error_type="processing_error",
+                                    message=f"Validation failed: {exc}",
+                                    severity=ValidationSeverity.FATAL,
+                                )
+                            ],
+                        )
+                    )
+
+            run_report = CVATValidationRunReport(
+                samples=reports,
+                statistics=CVATValidationRunReport.compute_statistics(reports),
+            )
+
+            output_path = self.output_dir / f"validation_report_{set_label}.json"
+            output_path.write_text(
+                run_report.model_dump_json(indent=2),
+                encoding="utf-8",
+            )
+            _log.info(
+                "✓ Regenerated %s validation report with %d sample(s): %s",
+                set_label,
+                len(reports),
+                output_path,
+            )
+
     def create_ground_truth_dataset(self) -> None:
         """
         Step 1: Create ground truth dataset from CVAT folder exports.

docling_eval/campaign_tools/evaluate_cvat_tables.py

@@ -13,6 +13,7 @@
 
 from docling_eval.cvat_tools.document import DocumentStructure
 from docling_eval.cvat_tools.models import CVATElement, TableStructLabel
+from docling_eval.cvat_tools.parser import ParsedCVATFile, parse_cvat_file
 
 DEFAULT_TABLE_PAIR_IOU: float = 0.20
 DEFAULT_CONTAINMENT_THRESH: float = 0.50
@@ -296,10 +297,19 @@ def evaluate_image(
     containment_thresh: float,
     table_pair_iou: float,
     sem_match_iou: float,
+    *,
+    parsed_set_a: Optional[ParsedCVATFile] = None,
+    parsed_set_b: Optional[ParsedCVATFile] = None,
 ) -> Optional[ImageTablesEvaluation]:
     try:
-        doc_a = DocumentStructure.from_cvat_xml(set_a_xml, image_name)
-        doc_b = DocumentStructure.from_cvat_xml(set_b_xml, image_name)
+        parsed_a = (
+            parsed_set_a if parsed_set_a is not None else parse_cvat_file(set_a_xml)
+        )
+        parsed_b = (
+            parsed_set_b if parsed_set_b is not None else parse_cvat_file(set_b_xml)
+        )
+        doc_a = DocumentStructure.from_parsed_image(parsed_a.get_image(image_name))
+        doc_b = DocumentStructure.from_parsed_image(parsed_b.get_image(image_name))
     except Exception:
         return None
 
@@ -352,6 +362,8 @@ def evaluate_tables(
     Returns the full evaluation model (no file I/O, no Typer types).
     """
     imgs = sorted(set(list_images_in_xml(set_a)) & set(list_images_in_xml(set_b)))
+    parsed_set_a = parse_cvat_file(set_a)
+    parsed_set_b = parse_cvat_file(set_b)
     evals: list[ImageTablesEvaluation] = []
     for name in imgs:
         res = evaluate_image(
@@ -361,6 +373,8 @@ def evaluate_tables(
             containment_thresh=containment_thresh,
             table_pair_iou=table_pair_iou,
             sem_match_iou=sem_match_iou,
+            parsed_set_a=parsed_set_a,
+            parsed_set_b=parsed_set_b,
         )
         if res is not None:
             evals.append(res)

docling_eval/campaign_tools/run_cvat_deliveries_pipeline.py

@@ -27,6 +27,7 @@ class ExecutionPlan:
     run_merge: bool
     run_dataset_creation: bool
     run_evaluation: bool
+    run_validation_reports: bool
     force_rerun: bool
     modalities: List[str]
 
@@ -35,6 +36,7 @@ def from_args(
         cls,
         merge_only: bool,
         eval_only: bool,
+        validation_only: bool,
         force: bool,
         modalities: Optional[Sequence[str]],
     ) -> ExecutionPlan:
@@ -52,13 +54,40 @@ def from_args(
         Raises:
             ValueError: If incompatible flags are combined
         """
-        if merge_only and eval_only:
-            raise ValueError("Cannot combine --merge-only and --eval-only")
+        exclusive_flags = [
+            flag for flag in (merge_only, eval_only, validation_only) if flag
+        ]
+        if len(exclusive_flags) > 1:
+            raise ValueError(
+                "Cannot combine --merge-only, --eval-only, or --validation-only"
+            )
+
+        if merge_only:
+            run_merge = True
+            run_dataset_creation = False
+            run_evaluation = False
+            run_validation_reports = False
+        elif eval_only:
+            run_merge = False
+            run_dataset_creation = False
+            run_evaluation = True
+            run_validation_reports = False
+        elif validation_only:
+            run_merge = False
+            run_dataset_creation = False
+            run_evaluation = False
+            run_validation_reports = True
+        else:
+            run_merge = True
+            run_dataset_creation = True
+            run_evaluation = True
+            run_validation_reports = False
 
         return cls(
-            run_merge=merge_only or not eval_only,
-            run_dataset_creation=not merge_only and not eval_only,
-            run_evaluation=not merge_only,
+            run_merge=run_merge,
+            run_dataset_creation=run_dataset_creation,
+            run_evaluation=run_evaluation,
+            run_validation_reports=run_validation_reports,
             force_rerun=force,
             modalities=(
                 list(modalities)
@@ -76,6 +105,11 @@ def should_skip_job(self, job: SubmissionSubsetJob) -> tuple[bool, str]:
         Returns:
             Tuple of (should_skip, reason_message)
         """
+        if self.run_validation_reports and not (
+            self.run_merge or self.run_dataset_creation or self.run_evaluation
+        ):
+            return False, ""
+
         if self.force_rerun:
             return False, ""
 
@@ -104,11 +138,19 @@ def get_description(self) -> str:
         """Get human-readable description of what will be executed."""
         if self.run_merge and not (self.run_dataset_creation or self.run_evaluation):
             return "merge annotations for"
+        if self.run_validation_reports and not (
+            self.run_merge or self.run_dataset_creation or self.run_evaluation
+        ):
+            return "regenerate validation reports for"
         elif not self.run_dataset_creation and self.run_evaluation:
             return "run evaluation for"
         else:
             return "evaluate"
 
+    def should_aggregate_validation(self) -> bool:
+        """Determine if validation reports should be aggregated."""
+        return self.run_evaluation or self.run_validation_reports
+
 
 @dataclass(frozen=True)
 class SubmissionSubsetJob:
@@ -173,6 +215,12 @@ def _execute_job(
         if not (plan.run_dataset_creation or plan.run_evaluation):
             return None
 
+    if plan.run_validation_reports:
+        pipeline.regenerate_validation_reports_from_merged(
+            merged_dir=job.get_merged_xml_dir(),
+        )
+        return None
+
     # Stage 2: Create datasets
     if plan.run_dataset_creation:
         pipeline.create_ground_truth_dataset()
@@ -328,6 +376,7 @@ def run_jobs(
     force: bool = False,
     merge_only: bool = False,
     eval_only: bool = False,
+    validation_only: bool = False,
     force_ocr: bool = False,
     ocr_scale: float = 1.0,
     storage_scale: float = 2.0,
@@ -341,6 +390,7 @@ def run_jobs(
     plan = ExecutionPlan.from_args(
         merge_only=merge_only,
         eval_only=eval_only,
+        validation_only=validation_only,
         force=force,
         modalities=modalities,
     )
@@ -431,7 +481,7 @@ def run_jobs(
                 _LOGGER.debug("Subset failure details", exc_info=True)
 
         # Aggregate validation reports across successfully completed subsets only
-        if plan.run_evaluation:
+        if plan.should_aggregate_validation():
             if completed_jobs:
                 _LOGGER.info(
                     "Aggregating validation reports for submission %s (%d/%d subsets completed)",
@@ -599,6 +649,11 @@ def parse_args(argv: Optional[Sequence[str]] = None) -> argparse.Namespace:
         action="store_true",
         help="Skip dataset creation and rerun only the evaluation stage.",
     )
+    parser.add_argument(
+        "--validation-only",
+        action="store_true",
+        help="Regenerate validation reports using existing merged annotations.",
+    )
     parser.add_argument(
         "--force-ocr",
         action="store_true",
@@ -641,6 +696,7 @@ def main(argv: Optional[Sequence[str]] = None) -> None:
             force=args.force,
             merge_only=args.merge_only,
             eval_only=args.eval_only,
+            validation_only=args.validation_only,
             force_ocr=args.force_ocr,
             ocr_scale=args.ocr_scale,
             storage_scale=args.storage_scale,

docling_eval/cli/cvat_validator_cli.py

@@ -1,22 +1,35 @@
 import argparse
 import json
 from pathlib import Path
-from typing import List, Tuple
+from typing import Dict, List, Tuple
 
 from ..cvat_tools.models import CVATValidationReport, CVATValidationRunReport
-from ..cvat_tools.parser import find_samples_in_directory, get_all_images_from_cvat_xml
+from ..cvat_tools.parser import (
+    ParsedCVATFile,
+    find_samples_in_directory,
+    get_all_images_from_cvat_xml,
+    parse_cvat_file,
+)
 from ..cvat_tools.validator import Validator, validate_cvat_sample
 
 
 def process_samples(samples: List[Tuple[str, Path, str]]) -> CVATValidationRunReport:
     """Process a list of samples and return a validation report."""
     validator = Validator()
     reports: List[CVATValidationReport] = []
+    parsed_cache: Dict[Path, ParsedCVATFile] = {}
 
     for sample_name, xml_path, image_filename in samples:
         try:
+            parsed_file = parsed_cache.get(xml_path)
+            if parsed_file is None:
+                parsed_file = parse_cvat_file(xml_path)
+                parsed_cache[xml_path] = parsed_file
             validated = validate_cvat_sample(
-                xml_path, image_filename, validator=validator
+                xml_path,
+                image_filename,
+                validator=validator,
+                parsed_file=parsed_file,
             )
             if validated.report.errors:
                 reports.append(validated.report)

docling_eval/cvat_tools/__init__.py

@@ -37,7 +37,13 @@
     CVATValidationRunReport,
     ValidationSeverity,
 )
-from .parser import find_samples_in_directory
+from .parser import (
+    ParsedCVATFile,
+    ParsedCVATImage,
+    find_samples_in_directory,
+    get_all_images_from_cvat_xml,
+    parse_cvat_file,
+)
 from .path_mappings import (
     PathMappings,
     associate_paths_to_containers,
@@ -89,6 +95,10 @@
     "ValidationSeverity",
     # Parser
     "find_samples_in_directory",
+    "get_all_images_from_cvat_xml",
+    "parse_cvat_file",
+    "ParsedCVATFile",
+    "ParsedCVATImage",
     # Tree
     "TreeNode",
     "build_containment_tree",