-
Notifications
You must be signed in to change notification settings - Fork 107
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
Resolve: Add progress bar to run_inference_algorithm #614
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #614 +/- ##
==========================================
+ Coverage 99.18% 99.22% +0.04%
==========================================
Files 57 57
Lines 2576 2581 +5
==========================================
+ Hits 2555 2561 +6
+ Misses 21 20 -1 ☔ View full report in Codecov by Sentry. |
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.
Add a kwarg that default to False, similar to
progress_bar: bool = False, |
@junpenglao I am trying to figure out why some of the tests are failing -- weird that the details say the test is failing in a file that does not use Also, should I adapt some tests to use the progress_bar? |
Looks like there is a flaky test - could you change the rng_key of that test until it pass, and add a comment that it is a flaky test? |
@reubenharry found a bug in the function, cause by this line: Line 174 in 540db41
Could you remove this line and add a test? Something like: import jax
import jax.numpy as jnp
from blackjax.mcmc.hmc import hmc
from blackjax.util import run_inference_algorithm
def logdensity_fn(x):
return -0.5 * jnp.sum(jnp.square(x))
alg = hmc(
logdensity_fn=logdensity_fn,
inverse_mass_matrix=jnp.eye(2),
step_size=1.0,
num_integration_steps=1000,
)
_ = run_inference_algorithm(
rng_key=jax.random.PRNGKey(0),
initial_state_or_position=jnp.array([1.0, 1.0]),
inference_algorithm=alg,
num_steps=10,
progress_bar=True) |
@junpenglao I think you meant to tag me instead of @reubenharry? But yes definitely!, sorry about that. Should I start a new tests file, maybe |
test_util.py sounds good. |
@junpenglao I have to step away for the rest of the day. Just added a test for |
The flaky test is not failing now, so I am going to go ahead and merge it. Thanks! |
* Add progress bar to run_inference_algorithm * Add progress_bar as kwarg defaulting to False * Fix bug in run_inference_algorithm and add test --------- Co-authored-by: Paul Scemama <pscemama@mitre.org>
Closes #610. Adds progress bar to
run_inference_algorithm
.A few important guidelines and requirements before we can merge your PR:
main
commit;pre-commit
is installed and configured on your machine, and you ran it before opening the PR;Consider opening a Draft PR if your work is still in progress but you would like some feedback from other contributors.