Skip to content

Test set x deprecation - #1946

Merged
dgedon merged 18 commits into
sbi-dev:gsoc-2026from
Jocho-Smith:test-set-x-deprecation
Jul 24, 2026
Merged

Test set x deprecation#1946
dgedon merged 18 commits into
sbi-dev:gsoc-2026from
Jocho-Smith:test-set-x-deprecation

Conversation

@Jocho-Smith

@Jocho-Smith Jocho-Smith commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

This PR continue #1945 it was about making bind independent of set_x. This one prepares deprecation of set_x by 'flipping its role with bind'. By that I mean the following: in #1943 I used set_x inside bind, to build a stateless version of set_x. In #1945 I made them independent and here I will write set_x in a way that it uses bind internally. This way deleting set_x will be trivially safe.

Jocho-Smith and others added 13 commits July 22, 2026 07:33
- Add bind() method to BasePotential (abstract) and CustomPotentialWrapper
- Add bind() to LikelihoodBasedPotential, PosteriorBasedPotential, RatioBasedPotential
- Add bind() to VectorFieldBasedPotential with full parameter support
- Add bind() to EnsemblePotential in ensemble_posterior.py
- Add bind() to ConditionedPotential in conditional_density_utils.py
- Fix _base_recursor to use local variable _active_holder to avoid Python
  scoping issues with the _active parameter
- Update posteriors to use bind() instead of set_x() for immutability
- Fix PytorchReturnTypeWrapper, MultipleIndependent, OneDimPriorWrapper init order
  and add __deepcopy__ methods for proper deepcopy support
- Update error messages to reference bind() instead of set_x()
- Add bind() to test FakePotential and TractablePotential classes
- Add bind() method to BasePotential (abstract) and CustomPotentialWrapper
- Add bind() to LikelihoodBasedPotential, PosteriorBasedPotential, RatioBasedPotential
- Add bind() to VectorFieldBasedPotential with full parameter support
- Add bind() to EnsemblePotential in ensemble_posterior.py
- Add bind() to ConditionedPotential in conditional_density_utils.py
- Fix _base_recursor to use local variable _active_holder to avoid Python
  scoping issues with the _active parameter
- Update posteriors to use bind() instead of set_x() for immutability
- Fix PytorchReturnTypeWrapper, MultipleIndependent, OneDimPriorWrapper init order
  and add __deepcopy__ methods for proper deepcopy support
- Update error messages to reference bind() instead of set_x()
- Add bind() to test FakePotential and TractablePotential classes
- Replace remaining set_x calls in VectorFieldPosterior with bind
…d set_x inside vi_divergence; remove set_x usage inside bind()
@Jocho-Smith Jocho-Smith mentioned this pull request Jul 22, 2026
1 task
@Jocho-Smith

Copy link
Copy Markdown
Contributor Author

Great this all works now :)

Next I will move over the tests that use set_x to bind so deleting set_x will not break any tests.

Resolved conflicts:
- potentials: keep HEAD (direct attribute set in bind)
- conditional_density_utils.py: keep HEAD (deprecation in set_x) + upstream (_x_is_iid fix)
- test files: keep HEAD (direct attribute set in bind)

Fixed duplicate bind() methods in vector_field_potential.py and ensemble_posterior.py
@codecov

codecov Bot commented Jul 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 42.42424% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.83%. Comparing base (587f922) to head (5247683).
⚠️ Report is 41 commits behind head on gsoc-2026.

Files with missing lines Patch % Lines
sbi/inference/potentials/base_potential.py 0.00% 5 Missing ⚠️
.../inference/potentials/posterior_based_potential.py 0.00% 5 Missing ⚠️
sbi/utils/conditional_density_utils.py 0.00% 5 Missing ⚠️
sbi/inference/posteriors/ensemble_posterior.py 20.00% 4 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##           gsoc-2026    #1946      +/-   ##
=============================================
- Coverage      87.99%   87.83%   -0.17%     
=============================================
  Files            144      144              
  Lines          13499    13759     +260     
=============================================
+ Hits           11879    12085     +206     
- Misses          1620     1674      +54     
Flag Coverage Δ
fast 81.60% <42.42%> (?)

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

Files with missing lines Coverage Δ
sbi/inference/potentials/vector_field_potential.py 92.30% <100.00%> (+1.32%) ⬆️
sbi/inference/posteriors/ensemble_posterior.py 73.88% <20.00%> (-6.74%) ⬇️
sbi/inference/potentials/base_potential.py 83.60% <0.00%> (-9.26%) ⬇️
.../inference/potentials/posterior_based_potential.py 89.39% <0.00%> (-6.97%) ⬇️
sbi/utils/conditional_density_utils.py 87.50% <0.00%> (-5.76%) ⬇️

... and 4 files with indirect coverage changes

@dgedon dgedon left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Within this PR there are many things that I flagged in #1945, so please make sure to update those first and merge them into here.


DEPRECATED: Use bind() instead. This method delegates to bind() internally.
"""
import warnings

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

imports on file level

Comment thread sbi/inference/posteriors/ensemble_posterior.py
import warnings

warnings.warn(
"set_x() is deprecated, use bind() instead",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

check other deprecation statements in the toolbox. Probably we want to add something saying that it will be removed in the future.

Comment thread sbi/inference/posteriors/ensemble_posterior.py

def bind(self, x_o: Tensor, x_is_iid: bool = False) -> "PosteriorBasedPotential":
"""Create new potential with x bound, without mutable state."""
from sbi.utils.user_input_checks import process_x

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

put on file level

import warnings

warnings.warn(
"set_x() is deprecated, use bind() instead",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

same as above

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

same comments as on the last PR. Please merge first

Comment thread tests/inference_on_device_test.py Outdated

def bind(self, x_o: torch.Tensor, x_is_iid: bool = True) -> "FakePotential":
"""Create new potential with x bound, without mutable state."""
from sbi.utils.user_input_checks import process_x

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

put on top level

Comment thread tests/vi_test.py Outdated
self, x_o: torch.Tensor, x_is_iid: bool = True
) -> "TractablePotential":
"""Create new potential with x bound, without mutable state."""
from sbi.utils.user_input_checks import process_x

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

file level

DEPRECATED: Use bind() instead. This method delegates to bind() internally.
It will be removed in a future release.
"""
import warnings

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Not yet addressed. If you decide against it, please let me know why or resolve the comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ah sorry, I overlooked that.

self.flow = self.rebuild_flow(**ode_kwargs)
elif self._x_o is not None:
self.flows = self.rebuild_flows_for_batch(**ode_kwargs)
import warnings

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

import on file level

Comment on lines +184 to +186
self.prior = prior
self.device = None
self.return_type = return_type

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why was that moved from like 10 lines lower?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

the line super().init(...) below had strange side effects with the new bind definition. Moving it before that fixed it.

@Jocho-Smith
Jocho-Smith requested a review from dgedon July 23, 2026 21:56
@dgedon
dgedon merged commit 8b39279 into sbi-dev:gsoc-2026 Jul 24, 2026
16 of 17 checks passed
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