diff --git a/benchmarks/test_patch_benchmarks.py b/benchmarks/test_patch_benchmarks.py index 525fb1bbe..6bd489514 100644 --- a/benchmarks/test_patch_benchmarks.py +++ b/benchmarks/test_patch_benchmarks.py @@ -7,7 +7,9 @@ import pytest import dascore as dc +from dascore.config import config_context from dascore.utils.patch import get_start_stop_step +from dascore.workflow.processor import _FINGERPRINTS @pytest.fixture(scope="module") @@ -428,3 +430,99 @@ def test_align_1d_shift_valid(self, patch_2d_with_1d_shift): """Benchmark 2D patch with 1D shift coordinate (300 shifts), valid mode.""" patch = patch_2d_with_1d_shift patch.align_to_coord(time="shift_time", mode="valid") + + +class TestIdentityOverhead: + """ + What maintaining the lineage ids costs. + + The charge is per operation -- canonicalizing the call and digesting + it -- so it is invisible next to real signal processing and plain + next to an operation which barely touches the data. Both ends are + timed, because it is the cheap end which decides whether the + `patch_provenance` knob is worth keeping. + + Repeating one call is the cheap case: `fingerprint_call` memoizes, so + the second identical call pays the lookup and not the digest. Real + loops vary their arguments, so the uncached case is timed too. + """ + + @pytest.fixture(scope="class") + def tiny_patch(self): + """The smallest patch worth having: all overhead, no work.""" + return dc.Patch( + data=np.ones((2, 2)), + coords={"distance": np.arange(2), "time": np.arange(2)}, + dims=("distance", "time"), + ) + + @pytest.fixture(scope="class") + def big_mask(self, example_patch): + """A mask the fingerprint has to hash, being an array parameter.""" + return np.asarray(example_patch.data) > 0.5 + + @pytest.fixture() + def ids_disabled(self): + """ + Turn the ids off around a benchmark, not inside it. + + Entering the context builds and validates a whole config, which + is not what the control is supposed to be measuring. + """ + with config_context(patch_provenance="disabled"): + yield + + @pytest.mark.benchmark + def test_identity_overhead_tiny_patch(self, tiny_patch): + """The charge on a call which does nothing else, memoized.""" + tiny_patch.transpose() + + @pytest.mark.benchmark + def test_identity_overhead_tiny_patch_disabled(self, tiny_patch, ids_disabled): + """The same call with the ids off, as the control.""" + tiny_patch.transpose() + + @pytest.mark.benchmark + def test_identity_overhead_uncached(self, tiny_patch): + """ + The charge with the memo missed, which is what a real loop pays. + + The cache is cleared rather than the arguments varied, so this + times the same call as the memoized benchmark above and the two + differ by the digest alone. + """ + _FINGERPRINTS.clear() + tiny_patch.transpose() + + @pytest.mark.benchmark + def test_identity_overhead_uncached_disabled(self, tiny_patch, ids_disabled): + """The control for the uncached charge, clearing included.""" + _FINGERPRINTS.clear() + tiny_patch.transpose() + + @pytest.mark.benchmark + def test_identity_overhead_array_argument(self, example_patch, big_mask): + """An array parameter is hashed, which is the one unflat cost.""" + example_patch.where(big_mask) + + @pytest.mark.benchmark + def test_identity_overhead_array_argument_disabled( + self, example_patch, big_mask, ids_disabled + ): + """The control: the same call without hashing the mask.""" + example_patch.where(big_mask) + + @pytest.mark.benchmark + def test_identity_overhead_real_work(self, example_patch): + """Next to actual filtering the charge should not be findable.""" + example_patch.pass_filter(time=(10, 100)) + + @pytest.mark.benchmark + def test_identity_overhead_real_work_disabled(self, example_patch, ids_disabled): + """The control for real work.""" + example_patch.pass_filter(time=(10, 100)) + + @pytest.mark.benchmark + def test_processor_fingerprint(self, example_patch): + """Building an operation and asking it what it is.""" + dc.proc.normalize.op(dim="time").fingerprint diff --git a/docs/tutorial/patch.qmd b/docs/tutorial/patch.qmd index 6c7b03840..6c742b28a 100644 --- a/docs/tutorial/patch.qmd +++ b/docs/tutorial/patch.qmd @@ -319,6 +319,29 @@ The path is part of the id, so a derived id is not stable across machines. A sto An operation which hands the patch straight back did nothing, and records nothing. Neither id is part of `Patch.equals`: two patches holding the same data are equal however they were made. +### What builds a patch, and what operates on one + +`new`, `update` and `update_attrs` carry both ids through, unless you name one and set it yourself. They are not operations — they are how a patch function assembles its own result — so stamping there would count every operation twice. + +That has a consequence worth knowing: changing data through `new` yourself leaves the ids saying the data is unchanged and the route is the same one. + +```{python} +doubled = patch.new(data=patch.data * 2) + +# It says it is the same data by the same route, because nothing told it otherwise. +assert doubled.attrs.patch_id == patch.attrs.patch_id +assert doubled.attrs.processing_id == patch.attrs.processing_id +``` + +The ids describe what DASCore was asked to do. Work done through [patch functions](processing.qmd) is described; work done by reaching past them is not. Building a patch from arrays rather than from another patch is the honest case, and mints a new `patch_id`: + +```{python} +import numpy as np + +built = dc.Patch(data=np.asarray(patch.data), coords=patch.coords, dims=patch.dims) +assert built.attrs.patch_id != patch.attrs.patch_id +``` + Set `patch_provenance="disabled"` to stop maintaining them: ```{python} diff --git a/scripts/differential_check.py b/scripts/differential_check.py index 30c687f8d..61d4ec608 100644 --- a/scripts/differential_check.py +++ b/scripts/differential_check.py @@ -259,8 +259,13 @@ def digest(patch) -> dict: name: _hash(patch.get_array(name)) for name in sorted(patch.coords.coord_map) } # History holds the repr of the arguments, which says nothing about the - # answer, so it is left out. - attrs = patch.attrs.model_dump(exclude={"history", "coords"}) + # answer, so it is left out. `patch_id` goes with it for a harder + # reason: this dumps in two processes, and a patch not read from a + # file mints one, so every patch would differ and the check would say + # nothing. `processing_id` stays -- it is a digest of the route, the + # same in both processes, so it catches a call which stopped being + # stamped or started fingerprinting its arguments differently. + attrs = patch.attrs.model_dump(exclude={"history", "coords", "patch_id"}) return { "dtype": str(data.dtype), "shape": list(data.shape), diff --git a/tests/test_workflow/test_identity.py b/tests/test_workflow/test_identity.py index 23dfafa2e..45119e5dd 100644 --- a/tests/test_workflow/test_identity.py +++ b/tests/test_workflow/test_identity.py @@ -272,6 +272,40 @@ def test_two_patches_are_two_data(self): first = dc.get_example_patch("random_das") assert first.attrs.patch_id != dc.get_example_patch("random_das").attrs.patch_id + @pytest.mark.parametrize( + "builder", + [ + lambda p: p.new(data=p.data), + lambda p: p.new(data=np.asarray(p.data) * 2), + lambda p: p.update(data=np.asarray(p.data) * 2), + lambda p: p.update_attrs(tag="rebuilt"), + ], + ) + def test_building_a_patch_is_not_operating_on_one(self, patch, builder): + """ + `new` and `update` carry both ids through untouched. + + They are how a patch function assembles its own result, so + stamping here would count every operation twice. The cost is that + changing data through `new` yourself leaves the ids saying it did + not change -- documented in the patch tutorial, and pinned here + because it is a policy rather than an accident. + """ + # Operated on first: an unprocessed patch states no route, so + # preserving it would be satisfied by dropping it. + processed = patch.abs() + assert processed.attrs.processing_id != NOTHING_DONE + out = builder(processed) + assert out.attrs.patch_id == processed.attrs.patch_id + assert out.attrs.processing_id == processed.attrs.processing_id + + def test_a_patch_built_from_arrays_is_new_data(self, patch): + """Naming no source, it is not the same data as anything else.""" + built = dc.Patch( + data=np.asarray(patch.data), coords=patch.coords, dims=patch.dims + ) + assert built.attrs.patch_id != patch.attrs.patch_id + def test_an_operation_advances_what_was_done(self, patch): """Which is what the id is for.""" out = patch.normalize("time")