-
Notifications
You must be signed in to change notification settings - Fork 327
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bayesian SINDy #440
Bayesian SINDy #440
Conversation
Thank you so much for this - I've been interested in getting Hirsch et al's spike slab priors into pysindy for some time! And selfishly, I've wanted a reason to start interacting with jax. We implemented a bit of kernel smoothing from a paper of @jfelipeosorio in derivative, but we shied away from anything that required jax. Giving this a quick look now (but not yet a full review), here's some more engineering, less mathematical considerations:
Example
Code
Ok I lied, some math
|
Thanks for your comments and suggestions. I'm happy if this is useful! I have made a few changes:
I'll have a look at your other points when I have more time. |
Also, just as a point of order: This implementation uses the regularised horseshoe prior, not (yet) the spike and slab prior. The regularised horseshoe is computationally more convenient because it is differentiable, so you can just plug the whole shebang into a NUTS sampler. The spike and slab prior AFAIK requires a compound step sampler or some other clever way of making categorical moves. It's something I can look into, but I am not currently sure what is the best way to achieve this in |
Oh, sorry, yes I remember now. I meant to say horseshoe prior. I didn't mean to point you in a different direction. |
@Jacob-Stevens-Haas quick update. 2 new commits:
Still looking to update the notebook with the stuff you requested. |
@Jacob-Stevens-Haas Okay, I think this addresses all of your points. Please let me know if anything can be improved. I'm not sure what to add in terms of tests. The notebook has a |
Thank you! I'll go ahead and review it later this week. Before then, I'll figure out how to get it so your CI runs don't need manual approval. Sorry to keep you waiting. For tests,
|
Ok I added you as a contributor with "write" permission to the repo. You may need to click an invite link in an email, but that should enable CI to run automatically on your pushes. |
@Jacob-Stevens-Haas Thank you! The tests are failing when building the docs and I can't seem to figure out why. This is the trace:
I have already included |
The github workflow file creates a new environment and installs the current package, including all specified optional dependencies. You'd need to change this line to add your optional dependencies to the list. |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #440 +/- ##
==========================================
- Coverage 94.40% 94.40% -0.01%
==========================================
Files 37 38 +1
Lines 3989 4060 +71
==========================================
+ Hits 3766 3833 +67
- Misses 223 227 +4 ☔ View full report in Codecov by Sentry. |
Ah, I just found the invitation. It was on the front page of this repo 🤔 .. thank you! |
This is now added by refactoring the existing
This is slightly problematic, since if called outside a model context, the
This is now done as |
No, that makes sense. I always wonder about what the right way to test things things is (and thereby, understand), but having read more about reg horseshe, I agree the roundabout work is not necessary here. I read a bit more about the reg-horseshoe, so I'll add some more info in the docstrings back in in a suggested edit.
I meant even simpler setup, but more rigorous test, a la def test_sbr_fit():
x = np.eye(2)
y = np.array([[1], [1e-4]])
expected = np.array([[1], [0]])
result = SBR(num_warmup=10, num_samples=10).fit(x, y).coef_
np.testing.assert_allclose(result, expected, atol=1e-1) This is a good example of what it takes to get SBR to succeed in the simplest of cases, and I'm looking into why it's failing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All suggested docstring changes based on looking into parameters to get the test case to fit.
That's probably good enough for now. In an ideal world, we could have a test that kind of recreated canonical "horeshoe" in Figure 2 of Handling Sparsity via the Horseshoe or Figure 1 of Sparsity Information Regularization in the Horseshoe and other Shrinkage Priors. |
So that's all for testing. For the notebooks,
|
Co-authored-by: Jacob Stevens-Haas <[email protected]>
Co-authored-by: Jacob Stevens-Haas <[email protected]>
Co-authored-by: Jacob Stevens-Haas <[email protected]>
Co-authored-by: Jacob Stevens-Haas <[email protected]>
@Jacob-Stevens-Haas Thank you for keeping up with this.
This is why I avoided comparing to "theoretical" values for the test I originally wrote. MCMC will always require some samples to get a sensible answer, and I suspect that this test with 50 warmup and 50 samples is not yet a converged MCMC. But if this works, that is great.
Shrinkage does work as expected when running more elaborate examples, but parameters will hardly ever be exactly 0, since the I will have a look at converting the example to a script. |
All done. However |
LGTM, and thank you for your dedication on this! I'll look into that pytest issue separately - pytest allows you create command line arguments in conftest.py, which is what I did for Looks like example 19 runs in 8.5 seconds under test which is great :) |
Add Bayesian SINDy
Hi all!
This is a (possible) implementation of Bayesian SINDy. It uses
numpyro
under the hood to drive NUTS sampling.pysindy.optimizers
module,SBR
for Sparse Bayesian Regression.SBR
optimizer include hyperparameters to the statistical model, i.e.tau_0
,nu
ands
for the hyperprior andlamb
for the exponential prior over the measurement noise.numpyro.infer.MCMC
object.self.coef_
is set to the mean of the posterior samples.SINDy.optimizer.mcmc
object usingarviz
.This PR is a work in progress in that test and docs are missing. But I thought it would be better to put it in early in the process, in case anyone has questions, ideas or comments with regards to the implementation. Please let me know if this is a bad idea.
Thanks for considering this PR.
TODO: