-
-
Notifications
You must be signed in to change notification settings - Fork 126
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
Ordinal regression docs #719
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Hi @GStechschulte! I recommend to rebase so it runs the latest version of the CI :) Btw, is there anything I can do here to help you? |
Hey, and will do! Thanks :) Not necessarily. Although, there is a problem with ordinal models and the I need to see if marginaleffects supports ordinal models, and see if we can adopt their solution. |
Isn't it similar to the case of the |
@tomicapretto yes, it should be similar to the |
d06e32d
to
fb1724a
Compare
I think this PR is almost there. However, I am still questioning the interpretation of the threshold coefficients for the From my understanding, for ordinal models with the However, when I plot the logits of the coefficients for the sratio model, it appears as though they are “partially” cumulative (overall, the probability increases as category increases, but for some categories the probability decreases which is not possible under a cumulative specification). Plotting the PyMC graph for the sratio model, there are no constraints. |
Hi Gabriel! This is a fantastic example, it's not simple at all and you're making a great job. I left some comments with suggestions and thoughts. Also:
|
Thank you! It has been a fun notebook to implement 😄
Done.
Yes, it "feels" like it. I haven't ran any timed experiments though.
Agreed, since the ordering of the thresholds doesn't matter, doing something like Update: Changing the default threshold prior: response_level_n = len(attrition["YearsAtCompany"].unique())
mu = np.zeros(response_level_n - 1)
threshold_prior = {"threshold": bmb.Prior("Normal", mu=mu, sigma=1)}
sequence_model = bmb.Model(
"YearsAtCompany ~ 0 + TotalWorkingYears",
data=attrition,
family="sratio",
priors=threshold_prior
)
sequence_model
works as expected. For now, I am thinking I keep this and explain how to change the default prior for the thresholds. Then, we could open a separate PR? What do you think? |
I know it's more "tidy" to do it in a separate PR, but I think it's OK to do it in this one too. Anyway, I leave it up to you. Whatever you want is OK! |
1 similar comment
I know it's more "tidy" to do it in a separate PR, but I think it's OK to do it in this one too. Anyway, I leave it up to you. Whatever you want is OK! |
I implemented the zero mu vector in this PR 😄 and added a section in the notebook explaining the differences in the default priors for |
Codecov Report
@@ Coverage Diff @@
## main #719 +/- ##
=======================================
Coverage 89.56% 89.56%
=======================================
Files 44 44
Lines 3525 3525
=======================================
Hits 3157 3157
Misses 368 368
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
77d798f
to
e081f47
Compare
@@ -114,7 +114,7 @@ def scale_threshold(self): | |||
threshold = self.model.components["threshold"] | |||
if isinstance(threshold, ConstantComponent) and threshold.prior.auto_scale: | |||
response_level_n = len(np.unique(self.response_component.response_term.data)) | |||
mu = np.round(np.linspace(-2, 2, num=response_level_n - 1), 2) | |||
mu = np.zeros(response_level_n - 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If sequential models assume that for every response level there is a latent continuous variable mu
should be mu = response_level_n
and not response_level_n - 1
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ohh, I think you're right!
I'm thinking why the current approach is working and not failing. Is it because it's not considering the probability of Y being larger than the largest observed category? I think it would make sense for the years example, but I'm not sure if it would make sense for cases where there is a pre-specified set of categories.
I wrote that as I was looking at this visualization from the ordinal tutorial by Bürkner and Vuorre:
and I wonder: Do we always have a Pr(Y > K)? (as in the Y > 3 in the figure)
@GStechschulte if you make that modification and run the example, does it work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I remove the -1
, I get ValueError: Incompatible Elemwise input shapes [(35,), (36,)]
.
This makes sense as I stated in the docs because the sequential model is a product of probabilities, i.e., the probability that
In the case of the attrition dataset, there are 36 response categories. Because of the statement above, this makes sense why the probability of category 36 is 1. There is no category after 36, so once you multiply all of the previous probabilities with the current category, you get 1. Thus, you don't need a parameter (threshold) for it.
@GStechschulte on top of the comment that you raised, there are two nits. After that, feel free to merge. Excellent work!! edit: closed by accident haha |
ordinal models (cumulative and sratio)
* zero inflated poisson and hurdle poisson models * grammar fix and sort imports * interpret coeff. and model comparison section * code review changes * change wording in hurdle Poisson section * change posterior predictive bins to use np.arange
ordinal models (cumulative and sratio)
610b9b3
to
075f537
Compare
Git got me good on this one 😢 |
I'm not sure if I follow what happened. Do you need help? |
No haha. My local branch somehow diverged from this remote branch and I attempted to fix the conflicts manually. At the end, the easiest was to force push. By the way, thanks for the reviews! |
* ordinal model with cumulative link notebook * ordinal model with cumulative link function ordinal models (cumulative and sratio) * unified explanation for cumulative and sequential models * sratio model and data * code review changes * remove intercept in models * zero mu vector prior for sratio family * code review and add section on default priors * explicit explanation of K and k and added summary section * Zero inflated docs (bambinos#725) * zero inflated poisson and hurdle poisson models * grammar fix and sort imports * interpret coeff. and model comparison section * code review changes * change wording in hurdle Poisson section * change posterior predictive bins to use np.arange * ordinal model with cumulative link function ordinal models (cumulative and sratio) * use plot_ppc_discrete for posterior predictive samples * add plots explaining the ordinal outcome of the dataset --------- Co-authored-by: Gabriel Stechschulte <[email protected]>
This draft PR contains a notebook with an ordinal regression model for the newly added
cumulative
family.First, it is explained why ordered categorical outcomes require special treatment. Secondly, the basics of ordinal regression are explained, motivated through the use of the cumulative link function. Next, an intercept only model is fit to explain how the cumulative link function "describes" an ordered distribution. Lastly, a model with predictors is developed.
I will also add a section with the
sratio
family.Edit by Tomas Capretto: Closes #583