From 7a73648b4cf08bab35187463b38aef7c7eab89a0 Mon Sep 17 00:00:00 2001 From: Pierre Raybaut <1311787+PierreRaybaut@users.noreply.github.com> Date: Sun, 25 Oct 2026 08:28:33 +0100 Subject: [PATCH] fix(contracts): record the image ROI pixel convention in the call context Sigima 1.4 selects the pixels of image regions of interest by their centers, which changes the result of almost every image operation whose input carries regions of interest, while calls on images without ROIs are unchanged. A replay with Sigima 1.4 of such a call recorded with 1.3 reported a different result without telling why; increasing every image contract would have refused the replay of all image activities, ROI or not. The convention is now a rule of the execution context, like the X-alignment policy: sigima.image.roi.pixel_centers, version 1 (IMAGE_ROI_CONVENTION). describe_call returns it in a new "context" mapping, and prepare_execution in the prepared context, whenever an input image carries regions of interest (image_roi_convention). For a replay, prepare_execution refuses (unsupported_context) a call on images carrying regions of interest whose recorded context does not name this rule (recorded before Sigima 1.4) or names another version. Calls on images without ROIs and on signals are not concerned. DataLab-Capsule records the "context" mapping of the description since DataLab-Platform/DataLab-Capsule#4. The guidelines state that such rules, not the contract versions, are increased when a convention shared by all operations changes. Closes #89 Assisted-by: Claude Opus 5.5 --- doc/contributing/guidelines.rst | 7 ++ .../fr/LC_MESSAGES/contributing/guidelines.po | 3 + .../LC_MESSAGES/release_notes/release_1.04.po | 3 + doc/release_notes/release_1.04.md | 1 + sigima/proc/contracts/__init__.py | 10 ++- sigima/proc/contracts/execution.py | 83 +++++++++++++++++-- sigima/tests/common/execution_unit_test.py | 53 ++++++++++++ 7 files changed, 154 insertions(+), 6 deletions(-) diff --git a/doc/contributing/guidelines.rst b/doc/contributing/guidelines.rst index e7033e98..42c68300 100644 --- a/doc/contributing/guidelines.rst +++ b/doc/contributing/guidelines.rst @@ -109,6 +109,13 @@ Follow these rules when you add or change a computation function: fails explicitly), nor when the change is entirely covered by a rule recorded in the execution context (e.g. the X-alignment policy). +- Rules that apply to every operation are recorded in the execution context + rather than in each contract: the selection of image pixels by regions of + interest (:data:`sigima.proc.contracts.IMAGE_ROI_CONVENTION`, recorded when an + input image carries regions of interest). Increase the version of such a rule, + not the contract versions, when it changes: a replay refuses the calls + recorded with another version. + - The structure of the contract (input roles, parameter arguments, outputs) is inferred from the annotations. Annotate every argument and the return value with names imported at runtime (not only under ``TYPE_CHECKING``). When roles diff --git a/doc/locale/fr/LC_MESSAGES/contributing/guidelines.po b/doc/locale/fr/LC_MESSAGES/contributing/guidelines.po index 4e7bf4bc..478e8f4c 100644 --- a/doc/locale/fr/LC_MESSAGES/contributing/guidelines.po +++ b/doc/locale/fr/LC_MESSAGES/contributing/guidelines.po @@ -106,6 +106,9 @@ msgstr "Augmentez ``contract_version`` pour tout changement de sens : paramètre msgid "In particular, increase it whenever a call that succeeded may now give another result, even for unusual inputs only (e.g. mixed real and complex, or ``float32`` and ``float64``, data): a replay of an activity recorded with the earlier version is then refused explicitly instead of being reported as different. Do not increase it when only inputs that were refused are now accepted, or when inputs that were accepted are now refused (the replay then fails explicitly), nor when the change is entirely covered by a rule recorded in the execution context (e.g. the X-alignment policy)." msgstr "En particulier, augmentez-la dès qu'un appel qui réussissait peut désormais donner un autre résultat, même pour des entrées inhabituelles seulement (par exemple des données mêlant réels et complexes, ou ``float32`` et ``float64``) : le rejeu d'une activité enregistrée avec la version précédente est alors refusé explicitement au lieu d'être signalé comme différent. Ne l'augmentez pas lorsque seules des entrées auparavant refusées sont désormais acceptées, ou lorsque des entrées auparavant acceptées sont désormais refusées (le rejeu échoue alors explicitement), ni lorsque le changement est entièrement couvert par une règle enregistrée dans le contexte d'exécution (par exemple la politique d'alignement en X)." +msgid "Rules that apply to every operation are recorded in the execution context rather than in each contract: the selection of image pixels by regions of interest (:data:`sigima.proc.contracts.IMAGE_ROI_CONVENTION`, recorded when an input image carries regions of interest). Increase the version of such a rule, not the contract versions, when it changes: a replay refuses the calls recorded with another version." +msgstr "Les règles qui s'appliquent à toutes les opérations sont enregistrées dans le contexte d'exécution plutôt que dans chaque contrat : la sélection des pixels d'une image par les régions d'intérêt (:data:`sigima.proc.contracts.IMAGE_ROI_CONVENTION`, enregistrée lorsqu'une image d'entrée porte des régions d'intérêt). Augmentez la version d'une telle règle, et non les versions des contrats, lorsqu'elle change : un rejeu refuse les appels enregistrés avec une autre version." + msgid "The structure of the contract (input roles, parameter arguments, outputs) is inferred from the annotations. Annotate every argument and the return value with names imported at runtime (not only under ``TYPE_CHECKING``). When roles cannot be inferred (more than two input objects), declare them with ``roles=(...)``." msgstr "La structure du contrat (rôles des entrées, arguments de paramètres, sorties) est déduite des annotations. Annotez chaque argument et la valeur de retour avec des noms importés à l'exécution (pas seulement sous ``TYPE_CHECKING``). Lorsque les rôles ne peuvent pas être déduits (plus de deux objets en entrée), déclarez-les avec ``roles=(...)``." diff --git a/doc/locale/fr/LC_MESSAGES/release_notes/release_1.04.po b/doc/locale/fr/LC_MESSAGES/release_notes/release_1.04.po index cc9f6197..9f396f72 100644 --- a/doc/locale/fr/LC_MESSAGES/release_notes/release_1.04.po +++ b/doc/locale/fr/LC_MESSAGES/release_notes/release_1.04.po @@ -64,6 +64,9 @@ msgstr "**ROI rectangulaires en indices de pixels** : le rectangle physique d'un msgid "**ROIs saved with earlier versions**: The ROIs of objects saved with earlier versions are not converted: they select pixels according to the rules above, so that processing and analysis in these ROIs may give different results when they are recomputed." msgstr "**ROI enregistrées avec des versions antérieures** : les ROI des objets enregistrés avec des versions antérieures ne sont pas converties : elles sélectionnent les pixels selon les règles ci-dessus, de sorte que les traitements et analyses dans ces ROI peuvent donner des résultats différents lorsqu'ils sont recalculés." +msgid "**Replay of processing in image ROIs**: The selection of image pixels by regions of interest is now recorded with each call on images carrying regions of interest (rule `sigima.image.roi.pixel_centers`, `sigima.proc.contracts.IMAGE_ROI_CONVENTION`, in the execution context described by `describe_call` and `prepare_execution`). Applications that replay recorded processing (e.g. DataLab capsules) refuse to replay such calls recorded with earlier versions, which selected other pixels, instead of reporting a different result. Calls on images without regions of interest are not concerned." +msgstr "**Rejeu des traitements dans les ROI d'images** : la sélection des pixels d'une image par les régions d'intérêt est désormais enregistrée avec chaque appel sur des images portant des régions d'intérêt (règle `sigima.image.roi.pixel_centers`, `sigima.proc.contracts.IMAGE_ROI_CONVENTION`, dans le contexte d'exécution décrit par `describe_call` et `prepare_execution`). Les applications qui rejouent des traitements enregistrés (par exemple les capsules DataLab) refusent de rejouer de tels appels enregistrés avec des versions antérieures, qui sélectionnaient d'autres pixels, au lieu de signaler un résultat différent. Les appels sur des images sans région d'intérêt ne sont pas concernés." + msgid "**Image center**: `ImageObj.xc` and `ImageObj.yc` are now the center of the pixel grid (halfway between the centers of the first and last pixels), for uniform and non-uniform coordinates: they were half a pixel off for uniform coordinates. `width` and `height` are unchanged." msgstr "**Centre de l'image** : `ImageObj.xc` et `ImageObj.yc` sont désormais le centre de la grille de pixels (à mi-chemin entre les centres du premier et du dernier pixel), pour des coordonnées uniformes ou non : ils étaient décalés d'un demi-pixel pour des coordonnées uniformes. `width` et `height` sont inchangés." diff --git a/doc/release_notes/release_1.04.md b/doc/release_notes/release_1.04.md index d6fdcb02..6aa4b7c1 100644 --- a/doc/release_notes/release_1.04.md +++ b/doc/release_notes/release_1.04.md @@ -22,6 +22,7 @@ The [user guide](../user_guide/overview.rst) shows how readers and connectors at * **Grids of ROIs**: The cells of ROI grids are now made of whole pixels: each cell holds the pixels whose centers lie in it, from its start included to its end excluded, and its ROI goes from the outer edges of these pixels. Adjacent cells thus never share a pixel, and the cells of a grid with 100 % sizes and steps tile the image exactly; cell positions are rounded to whole pixels. The grid of an image with non-uniform coordinates now spans the actual extent of its axes: its last rows and columns were left out. * **Rectangular ROIs in pixel indices**: The physical rectangle of a ROI defined in pixel indices now goes from the outer edges of its pixels, half a pixel before the center of the first pixel and half a pixel after the center of the last one. Converting a ROI between pixel indices and physical coordinates keeps the same pixels. * **ROIs saved with earlier versions**: The ROIs of objects saved with earlier versions are not converted: they select pixels according to the rules above, so that processing and analysis in these ROIs may give different results when they are recomputed. +* **Replay of processing in image ROIs**: The selection of image pixels by regions of interest is now recorded with each call on images carrying regions of interest (rule `sigima.image.roi.pixel_centers`, `sigima.proc.contracts.IMAGE_ROI_CONVENTION`, in the execution context described by `describe_call` and `prepare_execution`). Applications that replay recorded processing (e.g. DataLab capsules) refuse to replay such calls recorded with earlier versions, which selected other pixels, instead of reporting a different result. Calls on images without regions of interest are not concerned. * **Image center**: `ImageObj.xc` and `ImageObj.yc` are now the center of the pixel grid (halfway between the centers of the first and last pixels), for uniform and non-uniform coordinates: they were half a pixel off for uniform coordinates. `width` and `height` are unchanged. * **Ellipse coordinates**: `EllipseCoordinates` (`sigima.objects.shape`) now holds the 5 values `(x, y, a, b, theta)` of ellipse geometry results, which can thus be transformed. * **Operation contracts**: The operations whose result changed for some inputs now declare version 2 of their contract: `sigima.signal.addition`, `average`, `standard_deviation`, `product`, `difference` and `signals_to_image` (inputs of different data types, uncertainty of a product), and `sigima.image.resampling`, `calibration`, `extract_roi`, `extract_rois` and `radial_profile` (pixel grid, complex data, pixels selected by their centers). Applications that replay recorded processing (e.g. DataLab capsules) now refuse to replay calls of these operations recorded with earlier versions, instead of reporting a different result. diff --git a/sigima/proc/contracts/__init__.py b/sigima/proc/contracts/__init__.py index 024336f6..722f107e 100644 --- a/sigima/proc/contracts/__init__.py +++ b/sigima/proc/contracts/__init__.py @@ -24,7 +24,9 @@ parameters actually used, rebuilt against the local classes only); - :mod:`~sigima.proc.contracts.execution`: description and preparation of calls, shared by ordinary processing and replay (:func:`describe_call`, - :func:`prepare_execution`, :func:`normalize_outputs`, :func:`with_roi`); + :func:`prepare_execution`, :func:`normalize_outputs`, :func:`with_roi`), and + the conventions recorded in the execution context + (:func:`image_roi_convention`); - :mod:`~sigima.proc.contracts.policies` and :mod:`~sigima.proc.contracts.alignment`: X-alignment policies of signal operations and their algorithms. @@ -72,11 +74,13 @@ ) from sigima.proc.contracts.execution import ( EXECUTION_ERROR_CODES, + IMAGE_ROI_CONVENTION, ExecutionError, PreparedExecution, default_roles, describe_call, execute, + image_roi_convention, implementation_of, normalize_outputs, prepare_execution, @@ -136,11 +140,13 @@ ), "execution": ( "EXECUTION_ERROR_CODES", + "IMAGE_ROI_CONVENTION", "ExecutionError", "PreparedExecution", "default_roles", "describe_call", "execute", + "image_roi_convention", "implementation_of", "normalize_outputs", "prepare_execution", @@ -168,6 +174,7 @@ "ARTIFACT_KINDS", "EXACT", "EXECUTION_ERROR_CODES", + "IMAGE_ROI_CONVENTION", "OBJECT_KINDS", "PARAMETER_CODEC_VERSION", "SMALLEST_GRID_LINEAR_HOLD", @@ -203,6 +210,7 @@ "execute", "get_operation_contract", "get_x_alignment_policy", + "image_roi_convention", "implementation_of", "normalize_outputs", "parameters_from_values", diff --git a/sigima/proc/contracts/execution.py b/sigima/proc/contracts/execution.py index c9600697..637cf91f 100644 --- a/sigima/proc/contracts/execution.py +++ b/sigima/proc/contracts/execution.py @@ -21,6 +21,18 @@ - :func:`with_roi` returns a copy of an input with recorded regions of interest, to replay a call on an input whose regions of interest changed since. +The execution context also records the conventions that the result depends on +whatever the operation. Rule ``sigima.image.roi.pixel_centers`` (version 1, +:data:`IMAGE_ROI_CONVENTION`), recorded under ``roi_convention`` when an input +image carries regions of interest: they select the pixels whose centers lie in +their shape, bounds included; the part of the image holding a region of +interest, the position of the results computed in it and the move of regions of +interest by flips, rotations, transposition and translation follow the same +pixels. Earlier Sigima versions (before 1.4) rounded bounds to the nearest pixel +indices: a replay refuses a call on images carrying regions of interest +recorded without this rule (``unsupported_context``) instead of giving another +result. + Errors carry a stable ``code`` (:class:`ExecutionError`); scientific refusals of a policy keep their own exception (:class:`~sigima.proc.contracts.alignment.XAlignmentError`) in ordinary processing @@ -33,6 +45,7 @@ import contextlib import dataclasses import sys +import types from collections.abc import Iterator, Sequence from importlib import metadata from typing import Any, Callable @@ -55,11 +68,13 @@ __all__ = [ "EXECUTION_ERROR_CODES", + "IMAGE_ROI_CONVENTION", "ExecutionError", "PreparedExecution", "default_roles", "describe_call", "execute", + "image_roi_convention", "implementation_of", "normalize_outputs", "prepare_execution", @@ -74,6 +89,12 @@ "unexpected_output", ) +#: Selection of image pixels by regions of interest: pixels whose centers lie in +#: their shape, bounds included (recorded under ``roi_convention``) +IMAGE_ROI_CONVENTION = types.MappingProxyType( + {"rule": "sigima.image.roi.pixel_centers", "version": 1} +) + _KINDS: tuple[tuple[type, str], ...] = ( (SignalObj, "signal"), (ImageObj, "image"), @@ -117,6 +138,43 @@ def default_roles(count: int) -> list[str]: return ["sources"] * count +def image_roi_convention(inputs: Sequence[Any]) -> dict[str, Any] | None: + """Return the image ROI convention a call depends on, as recorded. + + Args: + inputs: Input objects of the call. + + Returns: + A copy of :data:`IMAGE_ROI_CONVENTION` if an input image carries regions + of interest, None otherwise. + """ + for obj in inputs: + if isinstance(obj, ImageObj) and obj.roi is not None and not obj.roi.is_empty(): + return dict(IMAGE_ROI_CONVENTION) + return None + + +def _check_image_roi_convention(context: dict[str, Any], inputs: list[Any]) -> None: + """Check that a replay on images with regions of interest recorded the rule. + + Raises: + ExecutionError: ``unsupported_context`` if an input image carries regions + of interest and the recorded rule is missing or another one. + """ + expected = image_roi_convention(inputs) + if expected is None or context.get("roi_convention") == expected: + return + recorded = context.get("roi_convention") + if recorded is None: + detail = ( + "the selection of image pixels in regions of interest was not recorded " + "(call recorded before Sigima 1.4, which selected other pixels)" + ) + else: + detail = f"unsupported image ROI convention {recorded!r}" + raise ExecutionError("unsupported_context", detail) + + def implementation_of(func: Callable) -> dict[str, Any]: """Return the informative implementation descriptor of *func*. @@ -190,7 +248,7 @@ def describe_call( Returns: ``{"operation", "qualification", "pattern", "roles", "parameters", - "parameter_codec", "options", "metadata_read", "limits", + "parameter_codec", "options", "metadata_read", "context", "limits", "implementation"}``: ``operation`` is ``{"id", "contract_version"}`` or None; ``qualification`` is ``{"status", "comparison"}`` (status ``qualified``, @@ -202,7 +260,10 @@ def describe_call( :func:`prepare_execution`), to be recorded in the execution context; ``metadata_read`` lists the metadata keys of the inputs read by the computation (``{"role", "position", "key"}``, *position* in *inputs*), - whose values an application records and checks before a replay. + whose values an application records and checks before a replay; + ``context`` holds the other entries to record in the execution context, + checked by :func:`prepare_execution` for a replay (``roi_convention``, + see :func:`image_roi_convention`). """ if contract is _MISSING: contract = contract_for_function(func) @@ -216,6 +277,7 @@ def describe_call( "parameter_codec": PARAMETER_CODEC_VERSION, "options": {}, "metadata_read": [], + "context": {}, "limits": limits, "implementation": implementation_of(func), } @@ -237,6 +299,9 @@ def describe_call( } description["pattern"] = contract.pattern description["options"] = _current_options(contract) + convention = image_roi_convention(inputs) + if convention is not None: + description["context"]["roi_convention"] = convention if _inputs_match(contract, inputs): description["roles"] = contract.input_role_names(len(inputs)) reads = dict(contract.reads_metadata) @@ -284,8 +349,9 @@ class PreparedExecution: args: Positional arguments: prepared inputs (a list input as a list), then parameter arguments. context: Execution context to record: ``x_alignment`` (record of the - policy applied, or None) and, when the computation reads global options, - ``options`` (their values). + policy applied, or None), when the computation reads global options, + ``options`` (their values) and, when an input image carries regions of + interest, ``roi_convention`` (:data:`IMAGE_ROI_CONVENTION`). options: Global option values applied while the function runs. """ @@ -369,7 +435,9 @@ def prepare_execution( Raises: ExecutionError: ``input_mismatch`` if the inputs do not fit the roles, ``invalid_parameters`` if the parameters cannot be decoded or do not fit, - ``unsupported_context`` if a recorded global option is missing. + ``unsupported_context`` if a recorded global option is missing, or if an + input image carries regions of interest and the recorded context does + not name the image ROI convention (:data:`IMAGE_ROI_CONVENTION`). XAlignmentError: If a recorded policy is unknown or the inputs cannot be aligned (scientific refusal, also in ordinary processing). """ @@ -388,6 +456,8 @@ def prepare_execution( else: arguments = _parameter_arguments(contract, param) replay = context is not None + if replay: + _check_image_roi_convention(context, list(inputs)) option_values: dict[str, Any] = {} if contract.reads_options: if replay: @@ -414,6 +484,9 @@ def prepare_execution( exec_context: dict[str, Any] = {"x_alignment": record} if option_values: exec_context["options"] = dict(option_values) + convention = image_roi_convention(inputs) + if convention is not None: + exec_context["roi_convention"] = convention return PreparedExecution( function=contract.function, args=_call_arguments(contract, prepared_inputs, arguments), diff --git a/sigima/tests/common/execution_unit_test.py b/sigima/tests/common/execution_unit_test.py index 327a113c..5665f54d 100644 --- a/sigima/tests/common/execution_unit_test.py +++ b/sigima/tests/common/execution_unit_test.py @@ -132,6 +132,7 @@ def test_describe_call_without_contract() -> None: assert description["qualification"]["status"] == "no_contract" assert description["roles"] == ["source"] assert description["parameters"] == {"method": "maximum"} + assert not description["context"] assert describe_call(no_contract, [src, src], roles=["a", "b"])["roles"] == [ "a", "b", @@ -140,6 +141,58 @@ def test_describe_call_without_contract() -> None: assert default_roles(3) == ["sources"] * 3 +def test_image_roi_convention_is_recorded_and_checked() -> None: + """Calls on images with ROIs record the pixel convention; replays require it.""" + rule = {"rule": "sigima.image.roi.pixel_centers", "version": 1} + assert dict(contracts.IMAGE_ROI_CONVENTION) == rule + contract = _contract(sigima.proc.image.absolute) + image = sigima.objects.create_image("i", np.arange(-32.0, 32.0).reshape(8, 8)) + # Without ROI, nothing is recorded nor required + assert not describe_call(contract.function, [image])["context"] + ordinary = prepare_execution(contract, [image], param=None) + assert ordinary.context == {"x_alignment": None} + prepare_execution(contract, [image], values={}, context={}) + # With a ROI, the rule is recorded by both the description and the preparation + image.roi = sigima.objects.create_image_roi("rectangle", [1, 1, 4, 4]) + description = describe_call(contract.function, [image]) + assert description["context"] == {"roi_convention": rule} + assert _json(description) == description + ordinary = prepare_execution(contract, [image], param=None) + assert ordinary.context == {"x_alignment": None, "roi_convention": rule} + reference = ordinary.run() + replay = prepare_execution(contract, [image], values={}, context=ordinary.context) + assert np.array_equal(replay.run().data, reference.data) + # Calls recorded without the rule (before Sigima 1.4) or with another one + for recorded in ( + {"x_alignment": None}, + {"roi_convention": None}, + {"roi_convention": {"rule": "sigima.image.roi.pixel_centers", "version": 2}}, + {"roi_convention": {"rule": "other.rule", "version": 1}}, + ): + with pytest.raises(ExecutionError) as info: + prepare_execution(contract, [image], values={}, context=recorded) + assert info.value.code == "unsupported_context" + with pytest.raises(ExecutionError, match="before Sigima 1.4"): + prepare_execution(contract, [image], values={}, context={}) + # An empty ROI selects every pixel, as before: nothing is required + image.roi = sigima.objects.create_image_roi("rectangle", [1, 1, 4, 4]) + image.roi.single_rois.clear() + assert not describe_call(contract.function, [image])["context"] + # Any input image carrying ROIs (here the operand) makes the call depend on it + source = sigima.objects.create_image("s", np.ones((8, 8))) + operand = image.copy() + operand.roi = sigima.objects.create_image_roi("circle", [4, 4, 2]) + difference = _contract(sigima.proc.image.difference) + context = describe_call(difference.function, [source, operand])["context"] + assert context == {"roi_convention": rule} + # Signal ROIs are not concerned + signal = _signal([0, 1, 2, 3], [-2.0, 0.0, 1.0, 4.0]) + signal.roi = sigima.objects.create_signal_roi([[0.5, 2.5]]) + normalize = _contract(sigima.proc.signal.normalize) + assert not describe_call(normalize.function, [signal], None)["context"] + assert contracts.image_roi_convention([signal]) is None + + def test_ordinary_and_replay_give_the_same_result() -> None: """Ordinary processing and replay share the same preparation.""" src = _signal([0, 1, 2, 3], [-2.0, 0.0, 1.0, 4.0])