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

GP-Latent update for v4 #237

Closed
wants to merge 2 commits into from
Closed

Conversation

bwengals
Copy link
Collaborator

@bwengals bwengals commented Oct 7, 2021

Associated with #72, and is tied to PR in pymc-devs/pymc #5055.

The notebook was reran using the fixes from the v4 GP branch, so the examples will error if merged before pymc v4 is done. Also I tried to make the right changes to be aligned with the style guide, but please let me know if I missed anything!

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@review-notebook-app
Copy link

review-notebook-app bot commented Oct 19, 2021

View / edit / reply to this conversation on ReviewNB

OriolAbril commented on 2021-10-19T06:52:22Z
----------------------------------------------------------------

the tags are not working, I think it's because of the raw cell, try putting it right below the title, in the markdown cell as this syntax is markdown for sphinx


fonnesbeck commented on 2022-04-02T19:52:36Z
----------------------------------------------------------------

@bwengals are you able to fix this?

Copy link
Member

@bwengals are you able to fix this?


View entire conversation on ReviewNB

@fonnesbeck
Copy link
Member

Please re-run the notebook to get rid of the errors in the current render.

@fonnesbeck
Copy link
Member

The fit is not great on the last model. Maybe boost the tune interval to 2000?

@fonnesbeck
Copy link
Member

Are you using a bespoke invlogit instead of pm.math.invlogit because of stability issues?

@ricardoV94
Copy link
Member

Are you using a bespoke invlogit instead of pm.math.invlogit because of stability issues?

If that's the case, check if it is still needed. We have improved invlogit stability in Aesara. Also the Bernoulli can take logit_p directly

@fonnesbeck
Copy link
Member

@bwengals how close is this one to being ready?

@bwengals
Copy link
Collaborator Author

bwengals commented Jun 5, 2022

So sorry for missing these comments :( I did finish it up today, hopefully I addressed you guys' comments. Unfortunately I used the wrong email address for a few previous commits, then made a big git mess trying to change them. If you guys don't mind, I'm just going to open a new PR tomorrow...

@bwengals bwengals mentioned this pull request Jun 5, 2022
3 tasks
@bwengals
Copy link
Collaborator Author

bwengals commented Jun 5, 2022

Closing for #371

@bwengals bwengals closed this Jun 5, 2022
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.

4 participants