-
-
Notifications
You must be signed in to change notification settings - Fork 132
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
[ENH] Add the Option to Choose Number of Draws in PPD #871
base: main
Are you sure you want to change the base?
Conversation
@jgyasu thanks for the contribution! I want to check that I understand the logic. You're sub-setting the InferenceData object before doing any computation. Thus, everything that happens later, like computation of likelihood parameters and generation of predictive draws, is based on that number of draws. Is that correct? |
Two suggestions
|
Yes, right. All the computation remains same, they just run on the subset of the total draws. |
Sure, thanks! I will apply these changes and make a new commit. |
@jgyasu thanks for the additional changes. I asked another change, and I also realized we should add a test for this case. Just let me know if you need any guidance. But what we want is a test for this |
Thanks, I will make the changes requested. And yes, this is what I was thinking! I'll try to write the tests and if I face any problem, I'll let you know! |
Hi @tomicapretto I made the requested change and I was going through the class FitPredictParent:
def fit(self, model, **kwargs):
return model.fit(tune=TUNE, draws=DRAWS, **kwargs)
def predict_oos(self, model, idata, data=None):
# Reuse the original data
if data is None:
data = model.data.head()
return model.predict(idata, kind="response", data=data, inplace=False) Should I make an additional class or add an additional method changing the arguments in the |
@jgyasu thanks! You don't need to create a new class. The one you shared is a parent class used by classes for specific model families. In the bambi/tests/test_model_construction.py Line 506 in 6b66691
num_draws , you can check the number of draws in the returned idata matches the one you passed, etc.
|
Hi, I have written the tests but I was trying to test them on my system and I tried installing all the dependencies but I get some errors during the building of I am on linux and in an isolated conda environment, I get this error - https://gist.github.com/jgyasu/0c767099cfa201169a703b7b886d76d9 I am wondering if I am missing some steps during installation, I will appreciate any help. Thanks! |
Hey @jgyasu thanks for the error output. Sorry for the naive recommendation, but have you tried creating a new environment and installing everything from scratch? |
@jgyasu it's usually the case that it's painful to work with PyMC and PyTensor, especially when installing new things. What environment manager are you using? If you're using conda, it should be as easy as Assuming you are at the root of the bambi repo, you can do conda create --name bambi-dev python=3.11
conda activate bambi-dev
conda install -c conda-forge pymc
pip install -e . # to install bambi in editable mode
pip install -e .[dev] # dev deps
pip install -e .[jax] # jax deps unfortunately it's not possible to do something like |
Closes #739
This PR modifies the
predict
method in theModel
class such that it now accepts an optional argumentnum_draws
and then randomly selects the number of draws from the posterior predictive distribution, i.e,num_draws
from the total draws without replacement. If thenum_draws
is more than the total draws then it gives a warning and falls back to the total draws.pylint
black
I will appreciate help with the testing of the method with this new parameter.