Skip to content

fix: transform_to_unconstrained device mdn device handling (#1893)#1896

Merged
janfb merged 9 commits into
sbi-dev:mainfrom
BHARATH0153:fix-device-state-dict
Jul 7, 2026
Merged

fix: transform_to_unconstrained device mdn device handling (#1893)#1896
janfb merged 9 commits into
sbi-dev:mainfrom
BHARATH0153:fix-device-state-dict

Conversation

@BHARATH0153

@BHARATH0153 BHARATH0153 commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Fixes MDN part of #1893

Two changes:

  1. mcmc_transform bounded branch now moves transform tensor internals to the requested device (was CPU-only via biject_to)
  2. MixtureDensityEstimator._apply overridden so _prior_transform follows .to(), .cuda(), .mps() moves

- Move mcmc_transform tensor internals to target device after construction
  (bounded branch via biject_to was CPU-only)
- Override _apply in MixtureDensityEstimator so _prior_transform follows
  model.to(), .cuda(), .mps() device moves
- Add _move_transform_tensors_to_device helper that recursively walks
  the transform tree (ComposeTransform, IndependentTransform, etc.)
@BHARATH0153 BHARATH0153 force-pushed the fix-device-state-dict branch from 505bb34 to 7e2df57 Compare June 26, 2026 15:17
@codecov

codecov Bot commented Jun 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 57.14286% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.82%. Comparing base (dbf432b) to head (4a1e306).
⚠️ Report is 20 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
sbi/utils/sbiutils.py 52.77% 17 Missing ⚠️
...eural_nets/estimators/mixture_density_estimator.py 83.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1896      +/-   ##
==========================================
- Coverage   87.88%   87.82%   -0.07%     
==========================================
  Files         143      143              
  Lines       13387    13474      +87     
==========================================
+ Hits        11765    11833      +68     
- Misses       1622     1641      +19     
Flag Coverage Δ
fast 81.30% <57.14%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...eural_nets/estimators/mixture_density_estimator.py 91.47% <83.33%> (-0.29%) ⬇️
sbi/utils/sbiutils.py 84.98% <52.77%> (-3.45%) ⬇️

... and 1 file with indirect coverage changes

@BHARATH0153

BHARATH0153 commented Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

@janfb please review when you have time thanks! and anything needed

@janfb janfb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @BHARATH0153 , looks good! It's focused on MDN for now and it works nicely. I just suggest a bit of refactoring towards a more general _move_tensor function.

The zuko and state-dict problems are not fixed here, which is fine. We can do it in a follow up.

Comment thread sbi/utils/sbiutils.py Outdated
Comment thread sbi/neural_nets/estimators/mixture_density_estimator.py Outdated
@janfb janfb changed the title fix: transform_to_unconstrained device and state_dict (#1893) fix: transform_to_unconstrained device mdn device handling (#1893) Jun 30, 2026
@janfb

janfb commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

One more thing: could you add test coverage for this? Codecov has it at ~20% atm. The test should be @pytest.mark.gpu (it'll auto-skip locally and run on the GPU job, like the other tests in tests/inference_on_device_test.py): build an MDN with z_score_x="transform_to_unconstrained", est.to("cuda"), then assert log_prob runs and the transform's tensors are on CUDA or MPS.
Thanks!

…sform

Replace _move_transform_tensors_to_device with generic _apply_to_transform
that accepts a callable fn, not just a device string.
Drop unused bare-tensor branch from list/tuple walker (transforms only
hold sub-transforms in lists, never tensors).
Port MixtureDensityEstimator._move_tensors callers to the shared helper
and remove the now-duplicate static method.
Add @pytest.mark.gpu test verifying MDN transform tensors move with
.to() and that log_prob / sample run on the target device.
@BHARATH0153 BHARATH0153 requested a review from janfb July 2, 2026 03:47
BHARATH0153 added a commit to BHARATH0153/sbi that referenced this pull request Jul 4, 2026
@BHARATH0153 BHARATH0153 force-pushed the fix-device-state-dict branch 2 times, most recently from c98155e to 3d0e223 Compare July 4, 2026 05:25
@BHARATH0153

Copy link
Copy Markdown
Contributor Author

@janfb once look into this please

@janfb janfb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @BHARATH0153 , this is exactly the refactor I had in mind, the core code looks good now. Now there are only things on the new tests that need to be fixed, and one small cleanup, then we can merge soon.

Comment thread sbi/utils/sbiutils.py Outdated
Comment thread tests/inference_on_device_test.py
Comment thread tests/inference_on_device_test.py Outdated
@BHARATH0153 BHARATH0153 force-pushed the fix-device-state-dict branch from 8d73994 to 4a1e306 Compare July 7, 2026 10:26
@BHARATH0153 BHARATH0153 requested a review from janfb July 7, 2026 10:32
@BHARATH0153

Copy link
Copy Markdown
Contributor Author

@janfb thanks! for the review done please check.

@BHARATH0153 BHARATH0153 marked this pull request as draft July 7, 2026 10:33
@BHARATH0153 BHARATH0153 marked this pull request as ready for review July 7, 2026 10:34

@janfb janfb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, well done @BHARATH0153 ! 👏

@janfb janfb merged commit 8f99ec9 into sbi-dev:main Jul 7, 2026
29 checks passed
BHARATH0153 added a commit to BHARATH0153/sbi that referenced this pull request Jul 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants