Skip to content

Monotonic trees missing values #27630

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

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

samronsin
Copy link
Contributor

Reference Issues/PRs

Follow up on #13649 to allow monotonic constraints on tree-based models when missing values are present (introduced in #23595 and #26391).

What does this implement/fix? Explain your changes.

Update on tests and monotonic constraints checks to make them work with missing values.

Any other comments?

For trees built with missing values, I had to work around:

  • unexpected middle values (still not able to understand them, opening a debug branch to inspect trees better)
  • the clipping mechanism modifying the nodes upper and lower bounds and making bound checking fail in assert_nd_reg_tree_children_monotonic_bounded.

Current implementation works, but I'm not satisfied with having to deal with middle values outside of the bounds. Will investigate further.

@github-actions
Copy link

github-actions bot commented Oct 20, 2023

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

Generated for commit: 8d803ef. Link to the linter CI: here

@glemaitre
Copy link
Member

@samronsin if you want to easily solve the problem linked to the linter, I encourage you to install pre-commit:

pip install pre-commit
pre-commit install

Then, It will use the version used by the CI to make the reformat the file before committing.

@glemaitre glemaitre self-requested a review October 30, 2023 13:22
@glemaitre glemaitre removed their request for review October 17, 2024 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants