stan-dev / pystan

PyStan, a Python interface to Stan, a platform for statistical modeling. Documentation: https://pystan.readthedocs.io
ISC License
337 stars 58 forks source link

fix: Update `num_samples_saved` formula in `fit.py` #367

Closed ahartikainen closed 1 year ago

ahartikainen commented 1 year ago

Fixes #366

Fixes the formula to calculate num_samples_saved. Updates a test in tests/test_normal.py to hit this specific problem for both num_samples and num_warmup when save_warmup=True.

riddell-stan commented 1 year ago

Looks good to me. Let's fix the CI before merging though.

ahartikainen commented 1 year ago

This test does not raise any errors now. Is this due to update in httpstan or where in pystan code should it raise that error?

def test_bernoulli_sampling_invalid_argument(posterior):
    with pytest.raises(TypeError, match=r"'float' object cannot be interpreted as an integer"):
        posterior.sample(num_thin=2.0)

edit. Oh yeah, math.ceil makes the input an integer. Should we add an assert statement somewhere to check this?

riddell-stan commented 1 year ago

First, let's adjust that test so it's more obvious what the problem is: num_thin=2.5 would be even clearer.

Sure, add an isinstance check (with a ValueError exception if it's not an integer). I think doing that is cleaner than an assert.

riddell-stan commented 1 year ago

LGTM!