Skip to content

Commit 5ea4bad

Browse files
janfbclaude
andcommitted
simplify: collapse transform walkers to a single _apply_to_transform (#1893)
#1896 shipped two near-identical tree-walkers (_apply_to_transform and a test-only _transform_tensors collector). Keep a single self-contained _apply_to_transform in production and collect tensors via a test helper that reuses it, removing the test-only function and the visitor indirection from the production module. Behavior-preserving; device/dtype/walker tests still green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 6420f31 commit 5ea4bad

3 files changed

Lines changed: 39 additions & 67 deletions

File tree

sbi/utils/sbiutils.py

Lines changed: 9 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -811,19 +811,19 @@ def match_theta_and_x_batch_shapes(theta: Tensor, x: Tensor) -> Tuple[Tensor, Te
811811
return theta_repeated, x_repeated
812812

813813

814-
def _walk_transform_tensors(
815-
transform: TorchTransform, visit: Callable[[object, str, Tensor], None]
814+
def _apply_to_transform(
815+
transform: TorchTransform, fn: Callable[[Tensor], Tensor]
816816
) -> None:
817-
"""Walk a transform tree, calling ``visit(holder, key, tensor)`` per tensor.
817+
"""Apply fn to all tensors in a transform tree, in place.
818818
819-
Traverses ``ComposeTransform.parts``, ``IndependentTransform.base_transform``,
820-
etc., so callers can move (``.to()``) or collect the tensors held inside a
821-
transform, which are otherwise invisible to ``nn.Module._apply``.
819+
So ``.to()``/``.double()`` calls propagate into a transform held as a plain
820+
attribute (e.g. a prior transform on an estimator), which is otherwise
821+
invisible to ``nn.Module._apply``. Traverses ``ComposeTransform.parts``,
822+
``IndependentTransform.base_transform`` and nested inverses.
822823
823824
Args:
824825
transform: Root of the transform tree.
825-
visit: Callback invoked for each tensor with its holder object and
826-
attribute name, e.g. to reassign or accumulate it.
826+
fn: Callable applied to each tensor (e.g. ``lambda t: t.to(device)``).
827827
"""
828828
seen = set()
829829

@@ -833,7 +833,7 @@ def _walk(t):
833833
seen.add(id(t))
834834
for key, val in list(t.__dict__.items()):
835835
if isinstance(val, Tensor):
836-
visit(t, key, val)
836+
object.__setattr__(t, key, fn(val))
837837
elif isinstance(val, (list, tuple)):
838838
for item in val:
839839
if isinstance(item, TorchTransform):
@@ -844,41 +844,6 @@ def _walk(t):
844844
_walk(transform)
845845

846846

847-
def _apply_to_transform(
848-
transform: TorchTransform, fn: Callable[[Tensor], Tensor]
849-
) -> None:
850-
"""Apply fn to all tensors in a transform tree.
851-
852-
So ``.to()``/``.double()`` calls propagate into a transform held as a plain
853-
attribute (e.g. a prior transform on an estimator).
854-
855-
Args:
856-
transform: Root of the transform tree.
857-
fn: Callable applied to each tensor (e.g. ``lambda t: t.to(device)``).
858-
"""
859-
860-
def _move(holder, key, tensor):
861-
object.__setattr__(holder, key, fn(tensor))
862-
863-
_walk_transform_tensors(transform, _move)
864-
865-
866-
def _transform_tensors(transform: TorchTransform) -> List[Tensor]:
867-
"""Collect every tensor in a transform tree.
868-
869-
Args:
870-
transform: Root of the transform tree.
871-
872-
Returns:
873-
List of all tensors found in the transform tree.
874-
"""
875-
tensors: List[Tensor] = []
876-
_walk_transform_tensors(
877-
transform, lambda _holder, _key, tensor: tensors.append(tensor)
878-
)
879-
return tensors
880-
881-
882847
def mcmc_transform(
883848
prior: Distribution,
884849
num_prior_samples_for_zscoring: int = 1000,

tests/inference_on_device_test.py

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,25 @@
5858
)
5959

6060

61+
def _collect_transform_tensors(transform):
62+
"""Collect every tensor in a transform tree, reusing the production walk.
63+
64+
Test helper for the transform_to_unconstrained device/dtype checks: drives
65+
``_apply_to_transform`` with a recording identity fn so we don't duplicate the
66+
tree-walk in the tests.
67+
"""
68+
from sbi.utils.sbiutils import _apply_to_transform
69+
70+
collected = []
71+
72+
def _record(tensor):
73+
collected.append(tensor)
74+
return tensor
75+
76+
_apply_to_transform(transform, _record)
77+
return collected
78+
79+
6180
@pytest.mark.slow
6281
@pytest.mark.gpu
6382
@pytest.mark.parametrize(
@@ -897,15 +916,14 @@ def test_npe_pfn_on_device(prior_device):
897916
def test_mdn_device_transform():
898917
"""MDN with transform_to_unconstrained moves transform tensors on .to()."""
899918
from sbi.neural_nets.net_builders.mdn import build_mdn
900-
from sbi.utils.sbiutils import _transform_tensors
901919

902920
device = process_device("gpu")
903921
prior = BoxUniform(-2 * torch.ones(2), 2 * torch.ones(2))
904922
bx, by = prior.sample((512,)), torch.randn(512, 3)
905923
est = build_mdn(bx, by, z_score_x="transform_to_unconstrained", x_dist=prior)
906924
est.to(device)
907925

908-
transform_tensors = _transform_tensors(est._prior_transform)
926+
transform_tensors = _collect_transform_tensors(est._prior_transform)
909927
assert transform_tensors, "expected the prior transform to hold tensors"
910928
for t in transform_tensors:
911929
assert t.device.type == device.split(":")[0], (
@@ -928,15 +946,14 @@ def test_mdn_transform_follows_dtype():
928946
Runs on every CI (no GPU needed).
929947
"""
930948
from sbi.neural_nets.net_builders.mdn import build_mdn
931-
from sbi.utils.sbiutils import _transform_tensors
932949

933950
prior = BoxUniform(-2 * torch.ones(2), 2 * torch.ones(2))
934951
bx, by = prior.sample((256,)), torch.randn(256, 3)
935952
est = build_mdn(bx, by, z_score_x="transform_to_unconstrained", x_dist=prior)
936953

937954
est.double()
938955

939-
transform_tensors = _transform_tensors(est._prior_transform)
956+
transform_tensors = _collect_transform_tensors(est._prior_transform)
940957
assert transform_tensors, "expected the prior transform to hold tensors"
941958
assert all(t.dtype == torch.float64 for t in transform_tensors)
942959

@@ -950,15 +967,14 @@ def test_mdn_transform_follows_dtype():
950967
def test_zuko_device_transform():
951968
"""Zuko flow with transform_to_unconstrained moves transform tensors on .to()."""
952969
from sbi.neural_nets.net_builders.flow import build_zuko_maf
953-
from sbi.utils.sbiutils import _transform_tensors
954970

955971
device = process_device("gpu")
956972
prior = BoxUniform(-2 * torch.ones(2), 2 * torch.ones(2))
957973
bx, by = prior.sample((512,)), torch.randn(512, 3)
958974
est = build_zuko_maf(bx, by, z_score_x="transform_to_unconstrained", x_dist=prior)
959975
est.to(device)
960976

961-
transform_tensors = _transform_tensors(est._prior_transform)
977+
transform_tensors = _collect_transform_tensors(est._prior_transform)
962978
assert transform_tensors, "expected the prior transform to hold tensors"
963979
for t in transform_tensors:
964980
assert t.device.type == device.split(":")[0], (
@@ -982,14 +998,13 @@ def test_zuko_transform_follows_dtype():
982998
.double()/.to() cast would silently leave it on the old dtype/device.
983999
"""
9841000
from sbi.neural_nets.net_builders.flow import build_zuko_maf
985-
from sbi.utils.sbiutils import _transform_tensors
9861001

9871002
prior = BoxUniform(-2 * torch.ones(2), 2 * torch.ones(2))
9881003
bx, by = prior.sample((256,)), torch.randn(256, 3)
9891004
est = build_zuko_maf(bx, by, z_score_x="transform_to_unconstrained", x_dist=prior)
9901005

9911006
est.double()
9921007

993-
transform_tensors = _transform_tensors(est._prior_transform)
1008+
transform_tensors = _collect_transform_tensors(est._prior_transform)
9941009
assert transform_tensors, "expected the prior transform to hold tensors"
9951010
assert all(t.dtype == torch.float64 for t in transform_tensors)

tests/sbiutils_test.py

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -711,37 +711,29 @@ def test_mdn_transform_to_unconstrained():
711711
assert s.shape[0] == 10 and torch.isfinite(s).all()
712712

713713

714-
def test_walk_transform_tensors_reaches_nested_tensors():
715-
"""The shared transform walker reaches tensors behind lists, nesting and .inv.
714+
def test_apply_to_transform_reaches_nested_tensors():
715+
"""`_apply_to_transform` reaches tensors behind lists, nesting and .inv.
716716
717-
Guards `_walk_transform_tensors` (and its `_apply_to_transform` /
718-
`_transform_tensors` wrappers), which move a prior transform onto a device.
719717
The leaf `loc`/`scale` sit behind `_InverseTransform -> IndependentTransform ->
720718
ComposeTransform.parts (a list) -> AffineTransform`, so a walk that missed the
721-
list branch or the inverse would silently move nothing.
719+
list branch or the inverse would silently touch nothing. A dtype cast is the
720+
observable effect: both leaves must change.
722721
"""
723722
from torch.distributions.transforms import (
724723
AffineTransform,
725724
ComposeTransform,
726725
SigmoidTransform,
727726
)
728727

729-
from sbi.utils.sbiutils import _apply_to_transform, _transform_tensors
728+
from sbi.utils.sbiutils import _apply_to_transform
730729

731-
loc, scale = torch.zeros(3), torch.ones(3)
732-
affine = AffineTransform(loc, scale)
730+
affine = AffineTransform(torch.zeros(3), torch.ones(3))
733731
# matches the shape mcmc_transform produces for a bounded prior, then inverted.
734732
transform = IndependentTransform(
735733
ComposeTransform([SigmoidTransform(), affine]), 1
736734
).inv
737735

738-
# collector reaches exactly the two leaf tensors (same objects), not zero.
739-
tensors = _transform_tensors(transform)
740-
assert len(tensors) == 2
741-
assert any(t is loc for t in tensors)
742-
assert any(t is scale for t in tensors)
743-
744-
# apply reaches every tensor via the same walk (dtype cast as an observable op).
736+
assert affine.loc.dtype == torch.float32 and affine.scale.dtype == torch.float32
745737
_apply_to_transform(transform, lambda t: t.double())
746738
assert affine.loc.dtype == torch.float64
747739
assert affine.scale.dtype == torch.float64

0 commit comments

Comments
 (0)