-
-
Notifications
You must be signed in to change notification settings - Fork 25.9k
DOC Add link to plot_monotonic_constraints.py in ensemble examples #31471
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
DOC Add link to plot_monotonic_constraints.py in ensemble examples #31471
Conversation
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.
Hi @shivamchhuneja!
Thanks for making this PR! That looks quiet good. I would just suggest to move the reference into the section "Monotonic Constraints" that is explicitly treating constraints for histogram based boosting, starting from (currently) line 327 in ensemble.rst
. There's also already a link there to it in the Examples section, that you could then remove (your reference is better conveying the information of what to expect from the example).
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.
Thanks, @shivamchhuneja.
Now we only need to remove the other reference above, since the link fits better below.
On your other question:
Yes, the approach of contextualizing the examples would be very useful for users to know what to expect before clicking a link. It always depends a bit, because sometimes, the title of the example already says it all.
Also, if you are interested, copy pasting from the issue:
- we are looking for people who are willing to do some intense work to improve or merge some examples; these will be PRs that will be intensely discussed and thoroughly reviewed and will probably take several months; if this sounds good to you, please open an issue with a suggestion and maintainers will evaluate your idea
- this could look like DOC rework the example presenting the regularization path of Lasso, Lasso-LARS, and Elastic Net #29963 and DOC merging the examples related to OPTICS, DBSCAN, and HDBSCAN #29962
- we also have an open issue to discuss examples that can be removed: RFC remove some of our examples #27151
doc/modules/ensemble.rst
Outdated
gradient boosting, including how they can improve generalization when "domain knowledge | ||
is available", see |
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.
I forgot to mention that in my previous review, but I was wondering about the quotation marks and I think we can leave them out, since we don't want to communicate anything special with it here.
It's a bit nit picky, but we really try to keep the docs as clean as possible.
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.
Makes sense, will make this change and note it for the future
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.
All the changes are done with the current PR.
And I would be 100% more than happy to pick up the deeper items that you've shared about the doc rework, as well as improving the documentation quality.
I might end up opening an issue in the next few days on one of the items where I feel that I can add value. Happy to be here.
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.
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.
They look great, happy to pick both one by one - #21138 would require a deeper scope and flow so maybe I can pick that one up first.
I'll leave a comment on that so we can just scope it out once. let me know if that sounds good
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.
Yes, that's the way to go about that. 👍
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.
awesome did that
Co-authored-by: Stefanie Senger <91849487+StefanieSenger@users.noreply.github.com>
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.
That looks very good. Thank you again for your work, @shivamchhuneja.
@adrinjalali, do you think we can merge that?
I'm not Adrin, but pressing merge anyway :D |
Reference Issues/PRs
Towards #30621
What does this implement/fix? Explain your changes.
-Added plot_monotonic_constraints.py example link to ensemble methods, under histogram based gradient boost
Screenshot of local check:
The example showed how to apply monotonic constraints in gradient boosting, which is useful when domain knowledge points that a feature should have a consistent directional effect on the target variable.
For example: in a marketing model, if we know that increasing the number of ad clicks should not decrease the likelihood of a user converting, we can enforce a monotonic constraint to ensure the model respects this expected behavior.
(slightly bland example but I guess this would at least give an idea on why I picked this one up)