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

Integrate ic_scale rcParam #985

Closed
OriolAbril opened this issue Jan 5, 2020 · 4 comments · Fixed by #993
Closed

Integrate ic_scale rcParam #985

OriolAbril opened this issue Jan 5, 2020 · 4 comments · Fixed by #993

Comments

@OriolAbril
Copy link
Member

Tell us about it

stats.ic_scale has a default value in rcParams which is not yet used.

Part of #792

Thoughts on implementation

compare, waic and loo should have None as default for ic_scale and use the default set in rcParams. The implementation should be very similar to
stats.information_criterion integration with ic argument

@Shashankjain12
Copy link
Contributor

@OriolAbril Can you assign me this issue I would be happy to work on that

@OriolAbril
Copy link
Member Author

@Shashankjain12 you can work on that, there is no need to be assigned. It is great that you comment on the issue you plan to work on.

One useful piece of advise in order to make sure you are not working on the same topic as somebody else (this is especially useful now that many of you are working on ArviZ to familiarize yourselves with it before GSoC), is to start by commenting on the issue just like you did and then once you start coding, send the pull request right away (even though it will probably not be finished). As said in the contributing guide, if you add [WIP] to the PR title and indicate the issue it fixes, it shows that work is currently ongoing.

Regarding your comment on Gitter, it looks like you are confusing ic (acronym for information criterion) with ic_scale. Working on rcParams can be quite confusing because the name of the rcParam may not always match exactly the argument used when calling ArviZ functions. I will try to clarify the issue description below, sorry about that.

In this case, the two arg/rcParam pairs are ic/information_criterion and scale/ic_scale. These arguments are relevant for several functions like loo, waic, compare and plot_elpd. ic already uses None as default, which means that the used can customize the default using the stats.information_criterion rcParam. However, scale has "deviance" hardcoded as a default, instead of using the stats.ic_scale rcParam.

The aim of the issue is to update scale/ic_scale to mimic the behaviour of ic/information_criterion

@Shashankjain12
Copy link
Contributor

Sure @OriolAbril Thanks for sorting that out I will start working on this issue..👍

@Shashankjain12
Copy link
Contributor

@OriolAbril I am not able to understand why my Travis CLI fails with
Bash exited with code '1'. Could you please help me what I can do to remove this error and pass all the checks

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