Skip to content

Fix subnet samples (posterior_scale.T issue) - #282

Open
elcorto wants to merge 2 commits into
aleximmer:mainfrom
elcorto:feature-fix-subnet-samples
Open

elcorto wants to merge 2 commits into
aleximmer:mainfrom
elcorto:feature-fix-subnet-samples

Conversation

@elcorto

@elcorto elcorto commented Sep 10, 2025

Copy link
Copy Markdown
Contributor

Closes #274

This fixes the posterior_scale.T issue discusssed in #274 (FullSubnetLaplace.sample()) and adds tests to make sure FullSubnetLaplace.sample() and DiagSubnetLaplace.sample() do the right thing.

We also add cov sampling tests for all other Laplace variants in tests/test_baselaplace.py and tests/test_lllaplace.py.

In test_baselaplace.py and test_lllaplace.py, move sampling test code
below the functional variance block. Add testing empirical covariance
of weight posterior samples vs. Laplace weight posterior covariance.
Use posterior_scale.T, as discussed in
    aleximmer#274
@elcorto

elcorto commented Sep 10, 2025 •

Copy link
Copy Markdown
Contributor Author

I can reproduce the pytest-with-asdfghjkl fail locally when I install git+https://git@github.com/wiseodd/asdl@asdfghjkl as the action does. Then the max error is ~0.7 instead of the allowed 0.01:

tests/test_baselaplace.py::test_laplace_functionality[LowRankLaplace-classification] >>>>> tensor(0.7377)
FAILED
tests/test_baselaplace.py::test_laplace_functionality[LowRankLaplace-regression] >>>>> tensor(0.7272)
FAILED

but only the combo LowRankLaplace + the above asdfghjkl lib. Maybe one can xfail that by testing for LowRankLaplace and a specific version (range) of the library? At least that's a quick solution to make the (PR) tests pass. Solving the asdfghjkl issue is a separate topic I guess.

@elcorto

elcorto commented Sep 10, 2025

Copy link
Copy Markdown
Contributor Author

The lint fail

93 |     assert res != math.nan
   |                   ^^^^^^^^
94 |     assert isinstance(res, float)
95 |     assert res > 0
   |

PLW0177 Comparing against a NaN value; use `math.isnan` instead

is not due to changes in this PR. Also, I'm with ruff here :)

elcorto added a commit to elcorto/laplace that referenced this pull request Mar 27, 2026

This branch has not been deployed

No deployments
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.

Potential bug in FullSubnetLaplace.sample()

1 participant