Skip to content
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

New discrete choice #618

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mitch-at-orika
Copy link

@mitch-at-orika mitch-at-orika commented Dec 8, 2023

Testing one alternative utility not zero (just the constant)

Issue #617


📚 Documentation preview 📚: https://pymc-examples--618.org.readthedocs.build/en/618/

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Copy link

review-notebook-app bot commented Dec 18, 2023

View / edit / reply to this conversation on ReviewNB

NathanielF commented on 2023-12-18T18:37:48Z
----------------------------------------------------------------

typo - "When [constants] are included

Also, when introducing this clarifying distinction it might be worth saying how this is occasionally mistaken for the need to 0 out the entire equation for the outside good, but the actual identification requirements are not as strict see reference etc...


Copy link

review-notebook-app bot commented Dec 18, 2023

View / edit / reply to this conversation on ReviewNB

NathanielF commented on 2023-12-18T18:37:49Z
----------------------------------------------------------------

Line #18.            beta_ic * wide_heating_df["ic.hp"] + beta_oc * wide_heating_df["oc.hp"]

If you're changing this line you should remove of clarify the np.zeros line


Copy link

review-notebook-app bot commented Dec 18, 2023

View / edit / reply to this conversation on ReviewNB

NathanielF commented on 2023-12-18T18:37:50Z
----------------------------------------------------------------

Line #20.            0 + beta_ic * wide_heating_df["ic.hp"] + beta_oc * wide_heating_df["oc.hp"]

This is fine. I think but we should keep to this pattern in subsequent models then too.


Copy link

review-notebook-app bot commented Dec 18, 2023

View / edit / reply to this conversation on ReviewNB

NathanielF commented on 2023-12-18T18:37:51Z
----------------------------------------------------------------

Great! Both coefficients are negative. Immediately better and more interpretable out of the gate. More effective sample sizes too.


Copy link

review-notebook-app bot commented Dec 18, 2023

View / edit / reply to this conversation on ReviewNB

NathanielF commented on 2023-12-18T18:37:52Z
----------------------------------------------------------------

Interesting that there appears to be wider uncertainty now. I guess we have more free parameters and they just propagate. I think the right interpretation is to stress a bit more on the freedom to specify individual utility for goods that is unlocked in this approach to the models


Copy link

review-notebook-app bot commented Dec 18, 2023

View / edit / reply to this conversation on ReviewNB

NathanielF commented on 2023-12-18T18:37:52Z
----------------------------------------------------------------

Check that this specification makes sense after re-fit.


Copy link

review-notebook-app bot commented Dec 18, 2023

View / edit / reply to this conversation on ReviewNB

NathanielF commented on 2023-12-18T18:37:53Z
----------------------------------------------------------------

Maybe add a note under these model comparison plots about how the freedom of the utility specification drives our understanding about the various components of market demand, and how the bayesian model development workflow helps uncover and validate the influences on desire


Copy link

review-notebook-app bot commented Dec 18, 2023

View / edit / reply to this conversation on ReviewNB

NathanielF commented on 2023-12-18T18:37:54Z
----------------------------------------------------------------

I think we should change this model too, but there is an argument that it's worth show-casing the common "zero-out" method too. But if you leave it as is you should add a note to explain the different approach.


Copy link

review-notebook-app bot commented Dec 18, 2023

View / edit / reply to this conversation on ReviewNB

NathanielF commented on 2023-12-18T18:37:55Z
----------------------------------------------------------------

Not necessary but it might be nice to overlay the 50% HDI on top of the 95% hdi you see here. It's more Bayesian in spirit and shows the centre of the distributions better.


@NathanielF
Copy link
Contributor

Thanks for the PR @mitch-at-orika! Sorry it took me so long to get back to you - Christmas has been waaay too busy. I've added a few comments above. Mostly about deciding how consistent we want to be in updating the model specification and making sure the rest of the text reflects the new model specification.

@NathanielF
Copy link
Contributor

On the pre-commit checks it looks like you just need to reset the notebook and run all cells from the top.

@mjbaske
Copy link

mjbaske commented Sep 12, 2024

@NathanielF can this possible be resurected of got a few things about model eval and macfaddens rho squared I would like to add but dont know if the branch is now way too out of date?

@NathanielF
Copy link
Contributor

Hi @mjbaske !

There has not been any changes to the notebook that I know of, but the pymc examples repo itself has likely been updated. I think you would be better off starting a new branch if the updates you envision are different from the changes started here. Just to keep things cleaner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants