Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix pareto k threshold typo in reloo function #1580

Merged
merged 2 commits into from
Feb 24, 2021

Conversation

teddygroves
Copy link
Contributor

Description

I noticed that changing the k_thresh argument to the function reloo from the default value 0.7 had no effect. I think this is because the threshold is accidentally hard-coded as 0.7 inside the function.

This change fixes the apparent typo, replacing 0.7 with k_thresh.

Checklist

  • Follows official PR format
  • Includes new or updated tests to cover the new feature
    This function is so far untested and I didn't think it made sense to write a whole test before fixing this bug.
  • Code style correct (follows pylint and black guidelines)
  • Changes are listed in changelog

Copy link
Member

@OriolAbril OriolAbril left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! It is great to see people using reloo, I wasn't really expecting it actually. I myself have not used it too intensively.

Let me know if you have any feedback on the sampling wrappers or on reloo itself (other than the calculation of p_loo being plain wrong). Would you find is helpful if for example reloo passed both label and positional indexes to the sampling wrapper sel_observations method? Are the docs on sampling wrappers clear?

@teddygroves
Copy link
Contributor Author

No worries, thanks a lot for implementing this - it's really improving my modelling workflow.

I'm still getting used to the interface and don't have the expertise to help with the p_loo calculations but I'll certainly come back with feedback soon.

@OriolAbril
Copy link
Member

I'll certainly come back with feedback soon.

Feel free to contact here on GitHub or at either of Stan or PyMC3 discourses.

@OriolAbril OriolAbril merged commit cb4b9e4 into arviz-devs:main Feb 24, 2021
@OriolAbril
Copy link
Member

Thanks for the PR @teddygroves !

utkarsh-maheshwari pushed a commit to utkarsh-maheshwari/arviz that referenced this pull request May 27, 2021
* Bug: fix pareto k threshold typo in reloo function

* Update changelog
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants