diff --git a/packages/scratch-core/src/conversion/surface_comparison/models.py b/packages/scratch-core/src/conversion/surface_comparison/models.py index e3e77adf..9f147c56 100755 --- a/packages/scratch-core/src/conversion/surface_comparison/models.py +++ b/packages/scratch-core/src/conversion/surface_comparison/models.py @@ -132,6 +132,7 @@ class ComparisonParams(ConfigBaseModel): :param search_angle_min: Lower bound of rotation search range (degrees). :param search_angle_max: Upper bound of rotation search range (degrees). :param search_angle_step: Angular step size for the coarse rotation sweep (degrees). + :param plot: Whether to save comparison plots. The remaining fields configure the two search stages; see their descriptions. How images are resampled is fixed rather than configurable, and lives in conversion.surface_comparison.pipeline. @@ -149,6 +150,7 @@ class ComparisonParams(ConfigBaseModel): search_angle_min: float = -180.0 search_angle_max: float = 180.0 search_angle_step: float = Field(default=5.0, gt=0.0) + plot: bool = Field(default=True, description="Whether to save comparison plots.") coarse_target_size: int = Field( default=256, diff --git a/scripts/comparison_utils.py b/scripts/comparison_utils.py index 831efae8..28279705 100644 --- a/scripts/comparison_utils.py +++ b/scripts/comparison_utils.py @@ -76,14 +76,17 @@ class ComparisonEntry: row_index: int -def _build_body(entry: ComparisonEntry) -> dict[str, Any]: - """Build the API request body for a comparison.""" +def _build_body(entry: ComparisonEntry, plot: bool = False) -> dict[str, Any]: + """Build the API request body for a comparison. + + :param plot: ask the API to render and save comparison plots (impression marks only). + """ processed_ref = str(entry.mark_dir_ref) processed_comp = str(entry.mark_dir_comp) if isinstance(entry.mark_type, MarkImpressionType): # Build params with default values; cell_size is derived from mark_type at runtime - params = ComparisonParams().model_dump() + params = ComparisonParams(plot=plot).model_dump() return { "mark_dir_ref": processed_ref, "mark_dir_comp": processed_comp, diff --git a/scripts/convert_scores.py b/scripts/convert_scores.py index d80d66de..249ba04b 100644 --- a/scripts/convert_scores.py +++ b/scripts/convert_scores.py @@ -18,6 +18,9 @@ re-runs the rows that errored. The full result payloads are still saved to the usual ``/database/mark-comparison-results/_comparison_results`` folders (CSV mode) or ``/generated-comparison-results/...`` (generated mode). + +Plots are off by default; ``--plot`` downloads them as PNGs into those same +per-comparison folders, next to ``comparison_results.json``. """ import argparse @@ -70,11 +73,12 @@ class ScoreStatus(enum.Enum): FAILED_ERROR = "failed_error" -def calculate_score( # noqa: PLR0911 - entry: ComparisonEntry, cfg: ConversionConfig, existing: set[Path] +def calculate_score( # noqa: PLR0911, PLR0912 + entry: ComparisonEntry, cfg: ConversionConfig, existing: set[Path], plot: bool = False ) -> tuple[ScoreStatus, dict[str, Any] | None]: """Call the score endpoint for a single comparison pair. + :param plot: request comparison plots from the API and download them (impression marks only). :returns: a ``(status, result_dict_or_none)`` tuple. """ if entry.comparison_out in existing and not cfg.force: @@ -88,7 +92,7 @@ def calculate_score( # noqa: PLR0911 endpoint = f"processor/calculate-score-{category}" try: - result = _post_with_retry(f"{cfg.api_url}/{endpoint}", _build_body(entry)) + result = _post_with_retry(f"{cfg.api_url}/{endpoint}", _build_body(entry, plot=plot)) except requests.HTTPError as exc: status_code = exc.response.status_code if exc.response is not None else 0 if status_code == 422: # noqa: PLR2004 @@ -120,6 +124,10 @@ def calculate_score( # noqa: PLR0911 return ScoreStatus.FAILED_ERROR, {"error": detail} _save_result(entry, result=result) + if not plot: + # Nothing was rendered server-side, so there is nothing to fetch. + return ScoreStatus.COMPLETED, result + downloaded = download_urls(result.get("urls", result), entry.comparison_out, skip=()) if not downloaded: @@ -208,11 +216,11 @@ def _row_values(status: ScoreStatus, result: dict[str, Any] | None, task: CsvTas def score_and_record( - task: CsvTask, cfg: ConversionConfig, existing: set[Path], writer: ScoreWriter + task: CsvTask, cfg: ConversionConfig, existing: set[Path], writer: ScoreWriter, plot: bool = False ) -> tuple[ScoreStatus, dict[str, Any] | None]: """Score one pair and immediately write its row to the scored CSV.""" try: - status, result = calculate_score(task.entry, cfg, existing) + status, result = calculate_score(task.entry, cfg, existing, plot=plot) except Exception: # Still leave a trace in the CSV before letting run_parallel handle it. writer.record(task.mark_type, task.row.index, _row_values(ScoreStatus.FAILED_ERROR, None, task)) @@ -255,6 +263,7 @@ def run_score_conversion( flush_every: int = 1, retry_failed: bool = False, max_depth: int = 2, + plot: bool = False, ) -> None: """Score comparison pairs and write one scored, resumable CSV per mark type. @@ -265,6 +274,7 @@ def run_score_conversion( :param flush_every: rewrite a scored CSV after this many comparisons. :param retry_failed: re-run rows that ended in an error last time (CSV mode only). :param max_depth: how deep below an item folder to look for mark folders (CSV mode only). + :param plot: request comparison plots from the API and download them (impression marks only). """ header, tasks = get_tasks( cfg, limit=limit, seed=seed, csv_path=csv_path, base=csv_base, delimiter=delimiter, max_depth=max_depth @@ -293,7 +303,7 @@ def run_score_conversion( try: counts = _run_scoring( - ((t.task_id, score_and_record, (t, cfg, existing, writer)) for t in tasks), + ((t.task_id, score_and_record, (t, cfg, existing, writer, plot)) for t in tasks), [t.task_id for t in tasks], workers, ) @@ -326,6 +336,9 @@ def main() -> None: "--csv-flush-every", type=int, default=1, help="Rewrite a scored CSV after this many comparisons" ) parser.add_argument("--retry-failed", action="store_true", help="Re-run rows that errored on a previous run") + parser.add_argument( + "--plot", action="store_true", help="Save comparison plots (impression marks only; off by default)" + ) parser.add_argument( "--csv-max-depth", type=int, default=2, help="How deep below an item folder to look for mark folders" ) @@ -345,6 +358,7 @@ def main() -> None: flush_every=args.csv_flush_every, retry_failed=args.retry_failed, max_depth=args.csv_max_depth, + plot=args.plot, ) diff --git a/src/processors/router.py b/src/processors/router.py index 37ea9b2c..c85d5bcb 100644 --- a/src/processors/router.py +++ b/src/processors/router.py @@ -110,17 +110,18 @@ async def calculate_score_impression(impression_params: CalculateScoreImpression logger.debug("CMC is calculated") - save_impression_comparison_plots( - mark_ref=mark_ref_processed, - mark_comp=mark_comp_processed, - cmc_result=cmc_result, - comparison_params=impression_params.comparison_params, - working_dir=vault.resource_path, - files_to_save=ComparisonImpressionFiles, - metadata_reference=impression_params.metadata_reference, - metadata_compared=impression_params.metadata_compared, - ) - logger.debug(f"images saved in:{vault.resource_path}") + if impression_params.comparison_params.plot: + save_impression_comparison_plots( + mark_ref=mark_ref_processed, + mark_comp=mark_comp_processed, + cmc_result=cmc_result, + comparison_params=impression_params.comparison_params, + working_dir=vault.resource_path, + files_to_save=ComparisonImpressionFiles, + metadata_reference=impression_params.metadata_reference, + metadata_compared=impression_params.metadata_compared, + ) + logger.debug(f"images saved in:{vault.resource_path}") comparison_results = ComparisonImpressionMetrics( score=cmc_result.cmc_count, @@ -129,7 +130,9 @@ async def calculate_score_impression(impression_params: CalculateScoreImpression estimated_translation=cmc_result.estimated_translation, ) return ComparisonResponseImpression( - urls=ComparisonResponseImpressionURL.from_enum(enum=ComparisonImpressionFiles, base_url=vault.access_url), + urls=ComparisonResponseImpressionURL.from_enum(enum=ComparisonImpressionFiles, base_url=vault.access_url) + if impression_params.comparison_params.plot + else None, cells=list(cmc_result.cells), comparison_results=comparison_results, ) diff --git a/src/processors/schemas.py b/src/processors/schemas.py index cce51672..68796825 100644 --- a/src/processors/schemas.py +++ b/src/processors/schemas.py @@ -210,7 +210,10 @@ class ComparisonImpressionMetrics(BaseModelConfig): class ComparisonResponseImpression(URLContainer): - urls: ComparisonResponseImpressionURL + urls: ComparisonResponseImpressionURL | None = Field( + default=None, + description="URLs of the comparison plots, or null when plotting was disabled.", + ) cells: list[Cell] = Field( default_factory=list, description="Per-cell CMC results for use in LR calculation.", diff --git a/tests/processors/test_router.py b/tests/processors/test_router.py index cf0a8a59..ad60d375 100644 --- a/tests/processors/test_router.py +++ b/tests/processors/test_router.py @@ -209,6 +209,35 @@ def test_calculate_impression_mark( for key in expected_images: assert client.get(response_data["urls"][key]).headers["content-type"] == "image/png", f"{key} should be PNG" + @pytest.mark.integration + def test_calculate_impression_mark_without_plots( + self, + client: TestClient, + impression_mark_dirs: tuple[Path, Path], + ) -> None: + """Scores are still returned, but no plots are made, when comparison_params.plot is False.""" + # Arrange + mark_dir_ref, mark_dir_comp = impression_mark_dirs + json_data = CalculateScoreImpression( + mark_dir_ref=mark_dir_ref, + mark_dir_comp=mark_dir_comp, + comparison_params=_default_comparison_params().model_copy(update={"plot": False}), + metadata_reference=_dummy_metadata(), + metadata_compared=_dummy_metadata(), + ).model_dump(mode="json") + + # Act + with patch("processors.router.save_impression_comparison_plots") as save_plots: + response = client.post("/processor/" + ProcessorEndpoint.CALCULATE_SCORE_IMPRESSION, json=json_data) + + # Assert + assert response.status_code == HTTPStatus.OK, response.json() + save_plots.assert_not_called() + response_data = response.json() + assert response_data["urls"] is None, "no plot URLs should be returned" + assert len(response_data["cells"]) > 0 + assert response_data["comparison_results"]["score"] >= 0 + class TestMarkImpressionExceptionHandlers: """One test per exception type for the impression score endpoint."""