Closed miemiekurisu closed 1 year ago
I'm reading this book again these days and find some of the contents a bit outdated even in pymc3 version. So I plan to update it to pymc current version. I believe that I've tested these new codes in the file well, and it works well under the pymc current version.
Great, thanks @miemiekurisu. This is pymc4, correct?
Great, thanks @miemiekurisu. This is pymc4, correct?
Definitely, based on the pymc4. And I found some gaps between pymc4 and 3, different APIs, different usages, etc. But I'll try my best ヾ(≧▽≦*)o
Thanks for taking this on @miemiekurisu!
Update Chapter 5, some codes work, but I'm afraid it needs some review or check = ̄ω ̄=
@CamDavidsonPilon Can you enable reviewnb for this repo?
♪(´▽`) Finished Ch1-6 and plan to check codes again. And add some Arviz content to make it look more "pymc4". ★,°:.☆( ̄▽ ̄)/$:.°★ 。
Once we have reviewnb added looking forward to reviewing!
I've added reviewnb to the repo. It's my first time using it, so I'm not 100% where it's accessed by others. Can ya'll access this page?
I've added reviewnb to the repo. It's my first time using it, so I'm not 100% where it's accessed by others. Can ya'll access this page?
Affirmative~ ヾ(•ω•`)o
Hi @miemiekurisu Miemiekurisu,
I restarted reading Davidson-Pilon book last weekend, in parallel with Oswaldo Martin´s book, and both were written with previous versions of PyMC and I was struggling to replicate the results with the current code until I found your fork.
Thank you very much for the effort! And also for adding some Arviz steps, which since PyMC3 makes the data analysis and model development check much easier.
Running it locally or even in Colab, I realized that your running time in the sampling step, even using exactly the same sampler and settings was much faster than mine, do you have any advice on how to speed up it?
Thanks
@Slepetys Hi,glad you found it helpful q(≧▽≦q) I'm not so sure about the speed difference, maybe it is the hardware difference? I just use i3 12th gen to do this work. To be honest, I think it's so so slow in some cases.
I provide my hardware brief below:
OS: Ubuntu 22.04.1 LTS x86_64 Kernel: 5.15.0-46-generic Shell: bash 5.1.16 CPU: 12th Gen Intel i3-12100 (8) @ 5.500GHz GPU: Intel Device 4692 Memory: 6280MiB / 15744MiB
View / edit / reply to this conversation on ReviewNB
twiecki commented on 2022-09-30T13:18:45Z ----------------------------------------------------------------
This should be replaced by parameter.initval
.
miemiekurisu commented on 2022-11-08T09:05:14Z ----------------------------------------------------------------
Yeah, you're right, and in fact I've noticed this initval and test_value problem when I upgrade it to pymc4.
According to this URL: https://github.com/pymc-devs/pymc/issues/562#issuecomment-932146862
test_val is a misusing, initval parameter should be used.
I try to keep the original content as much as possible at first, so I used the
aesara.config.compute_test_value = 'warn'
to keep compatibility.
Maybe I should add some brif comments to explain this problem to previous version readers?
View / edit / reply to this conversation on ReviewNB
twiecki commented on 2022-09-30T13:18:45Z ----------------------------------------------------------------
Here as well. There is no more test_value
.
View / edit / reply to this conversation on ReviewNB
twiecki commented on 2022-09-30T13:18:46Z ----------------------------------------------------------------
import aesara.tensor as at
View / edit / reply to this conversation on ReviewNB
twiecki commented on 2022-09-30T13:18:47Z ----------------------------------------------------------------
replace theano -> aesara everywhere.
View / edit / reply to this conversation on ReviewNB
twiecki commented on 2022-09-30T13:18:47Z ----------------------------------------------------------------
again usage of test_value.
View / edit / reply to this conversation on ReviewNB
twiecki commented on 2022-09-30T13:18:48Z ----------------------------------------------------------------
esara.config.compute_test_value = 'off' you can probably remove this.
View / edit / reply to this conversation on ReviewNB
twiecki commented on 2022-09-30T13:18:49Z ----------------------------------------------------------------
instead of accessing test_value or initval, if you want to sample from a random-variable, you can do pm.draw(first_coin_flips)
.
And you shouldn't have to set initval here at all then.
miemiekurisu commented on 2022-12-03T11:03:25Z ----------------------------------------------------------------
Got it.
View / edit / reply to this conversation on ReviewNB
twiecki commented on 2022-09-30T13:18:50Z ----------------------------------------------------------------
initval should not be required.
View / edit / reply to this conversation on ReviewNB
twiecki commented on 2022-09-30T13:18:51Z ----------------------------------------------------------------
import aesara.tensor as at
View / edit / reply to this conversation on ReviewNB
twiecki commented on 2022-09-30T13:18:52Z ----------------------------------------------------------------
here I would just pm.draw()
.
View / edit / reply to this conversation on ReviewNB
twiecki commented on 2022-09-30T13:18:52Z ----------------------------------------------------------------
PyMC4 -> PyMC 4
View / edit / reply to this conversation on ReviewNB
twiecki commented on 2022-09-30T13:18:53Z ----------------------------------------------------------------
Unfortunately this chapter is quite out-of-date, but since we're just porting here it's oK.
miemiekurisu commented on 2022-12-03T12:10:02Z ----------------------------------------------------------------
LOL
View / edit / reply to this conversation on ReviewNB
twiecki commented on 2022-09-30T13:18:55Z ----------------------------------------------------------------
I thought the syntax to pm.Potential has changed and you need to pass pm.Potential(logp=logp)
.Which version of PyMC 4 is this?
miemiekurisu commented on 2022-12-29T03:49:45Z ----------------------------------------------------------------
PYMC4 version is 4.1.5, let me check the API again
miemiekurisu commented on 2022-12-29T04:24:35Z ----------------------------------------------------------------
Accroding to the last version here: pymc/model.py at main · pymc-devs/pymc (github.com)
def Potential(name, var, model=None): """Add an arbitrary factor potential to the model likelihood Parameters ---------- name: str var: PyTensor variables Returns ------- var: var, with name attribute """
View / edit / reply to this conversation on ReviewNB
twiecki commented on 2022-09-30T13:18:56Z ----------------------------------------------------------------
Try with just pm.sample()
.
View / edit / reply to this conversation on ReviewNB
twiecki commented on 2022-09-30T13:18:57Z ----------------------------------------------------------------
PyMC3 -> PyMC
View / edit / reply to this conversation on ReviewNB
twiecki commented on 2022-09-30T13:18:57Z ----------------------------------------------------------------
T -> at
View / edit / reply to this conversation on ReviewNB
twiecki commented on 2022-09-30T13:18:58Z ----------------------------------------------------------------
Maybe add a note that Wishart is discouraged these days and that people should use LKJ.
@miemiekurisu Sorry this took a while but your port looks really good. There are some minor things, mainly about the usage of test_values but those should be easy to fix.
View / edit / reply to this conversation on ReviewNB
aaronsl-hku commented on 2022-10-18T07:06:09Z ----------------------------------------------------------------
LaTeX failed to render for me. Maybe good idea to wrap it with $$
?
miemiekurisu commented on 2022-12-03T11:29:05Z ----------------------------------------------------------------
That's strange, I think it's a front end or page css problem. Sometimes I got this problem too, and it works properly after I refresh the notebook page serviral times.
twiecki commented on 2022-12-29T10:30:57Z ----------------------------------------------------------------
ReviewNB often fails but then it displays fine.
View / edit / reply to this conversation on ReviewNB
aaronsl-hku commented on 2022-10-18T07:06:10Z ----------------------------------------------------------------
Line #4. trace = pm.sample(25000, step=step)
PyMC has kwarg tune now for burning in. I suggest:
trace = pm.sample(25000, step=step, tune=2500)
and remove the slicing in the next cell.
Ur right, I noticed the kwarg at the next chapters, and also left some comments.
At first, I trid to keep the original contents as much as possible. :)
@miemiekurisu Sorry this took a while but your port looks really good. There are some minor things, mainly about the usage of test_values but those should be easy to fix.
Sorry for not getting back to you sooner, I'm tied up with a company project whole Oct. (/▽\) Luckily, the project will be done at the middle of Nov. I'll fix these review problms ASAP. (/ω\)
Yeah, you're right, and infact I've noticed this initval and test_value problem when I upgrade it to pymc4.
Accroding to this URL: https://github.com/pymc-devs/pymc/issues/562#issuecomment-932146862
test_val is a misusing, initval parameter should be used.
I try to keep the orignal content as much as possible at first, so I used the
aesara.config.compute_test_value = 'warn'
to keep compatibility.
Maybe I should add some brif comments to explain this problem to previous version readers?
View entire conversation on ReviewNB
Maybe I should add some brif comments to explain this problem to previous version readers?
Yes, I think that'd be helpful.
That's strange, I think it's a front end or page css problem. Sometimes I got this problem too, and it works properly after I refresh the notebook page serviral times.
View entire conversation on ReviewNB
Ur right, I noticed the kwarg at the next chapters, and also left some comments.
View entire conversation on ReviewNB
Check out this pull request on
See visual diffs & provide feedback on Jupyter Notebooks.
Powered by ReviewNB
Accroding to the last version here: pymc/model.py at main · pymc-devs/pymc (github.com)
def Potential(name, var, model=None): """Add an arbitrary factor potential to the model likelihood Parameters ---------- name: str var: PyTensor variables Returns ------- var: var, with name attribute """--- View entire conversation on ReviewNB
Accroding to the last version here: pymc/model.py at main · pymc-devs/pymc (github.com)
OK, I was confused. Looks good then!
What are the next steps here? would be great to get this merged.
What are the next steps here? would be great to get this merged.
Sorry for not getting back to you sooner ...... again 〒▽〒 I believe I had checked and solved most ReviewNB problems. I think it's ready to be merged (/≧▽≦)/
@miemiekurisu Great -- thanks!
@CamDavidsonPilon Want to merge?
I've been pretty absent from this discussion, but thank you @miemiekurisu and @twiecki for shepherding this in!
Update to pymc current version and add some Arviz content