Fix crash when monotone_constraints_method is set without monotone_constraints by Shreyas-S-809 · Pull Request #7203 · lightgbm-org/LightGBM

Shreyas-S-809 · 2026-03-16T16:21:02Z

Summary

This PR fixes a crash that occurs when monotone_constraints_method
is set to "intermediate" or "advanced" while
monotone_constraints is not provided.

Previously, this configuration could lead to a segmentation fault
during training when monotone constraint logic attempted to access
an empty constraints vector.

Fix

A validation check has been added in Config::CheckParamConflict()
to detect this configuration and gracefully handle it by:

emitting a warning
falling back to the "basic" monotone constraints method

This ensures that training proceeds safely without crashing.

Tests

A new unit test has been added to verify that training completes
successfully when monotone_constraints_method is provided without
monotone_constraints.

All existing Python package tests pass locally.

Example

params = {
    "monotone_constraints_method": "advanced"
}

…notone_constraints

jameslamb

Thanks for attempting this! Please see my initial suggestions.

jameslamb · 2026-03-19T03:55:41Z

tests/python_package_test/test_monotone_constraints.py

@@ -0,0 +1,28 @@
+import lightgbm as lgb


We don't want totally new test file here. Please, add a new test case around the existing monotone constraints tests here:

LightGBM/tests/python_package_test/test_engine.py

Line 2186 in 8e7f3dc

def test_monotone_constraints(test_with_categorical_variable):

When you do that, please:

follow the conventions you see in that file (e.g. using one of the dataset fixtures or other approaches that provides enough data to train a real model)

expand the test to check that monotone_constraints_method is still preserved in the Booster's params

check that the expected warning is emitted

example:

LightGBM/tests/python_package_test/test_engine.py

Line 4690 in 8e7f3dc

with pytest.warns(UserWarning, match="LightGBM will perform up to 4 boosting rounds"):

Ohh, sorry for that,

Removed standalone test file and integrate test into existing
test_engine.py following project conventions.

jameslamb · 2026-03-19T03:57:05Z

src/io/config.cpp

+        monotone_constraints_method.c_str());
+
+    monotone_constraints_method = "basic";
+  }


CheckParamConflict() seems like a good place to do a check like this, but I don't understand how changing this to "basic" avoids the error.

Can you please links to the place(s) in the code where the segfault originates? I think seeing the root cause of the problem will help us to evaluate possible fixes.

Thanks for the detailed feedback!

I've updated the tests to add a new test case in test_engine.py, following the existing structure and dataset utilities. The test verifies that:

a warning is emitted when monotone_constraints_method is set without monotone_constraints

training completes without crashing

the method falls back to "basic"

I've also run pre-commit run --all-files locally and fixed the linting issues.

Regarding the segfault: my understanding is that when monotone_constraints_method is set to "advanced" or "intermediate" but monotone_constraints is empty, later stages of training (in the tree learner logic) assume that constraints are available and attempt to access them. This leads to invalid memory access and causes a segmentation fault.

Falling back to "basic" avoids those code paths that rely on non-empty monotone constraints.

I will continue investigating to identify the exact location in the code where this invalid access occurs and will update the PR with more precise references.

Thanks for this, but yes I think you'll need to go a bit deeper and find the root cause of the segfaults. There may be other codepaths to reach that code, and we should fix this problem at the root (e.g., maybe we need to check some that some pointers are non-null or something).

If you're not confident that you can do that, then let me know and I can try to help when I have time.

Thanks, that makes sense — I agree we should fix this at the root.

I’ll start tracing where monotone_constraints is accessed during training to identify the exact source of the segfault.

Just to make sure I’m looking in the right place: would you expect the issue to originate in the tree learner logic (e.g., histogram construction or split finding), or somewhere earlier in parameter handling?

Also, are there any specific files or components you’d recommend focusing on for this?

Thanks again — this is really helpful.

You will have to figure that out.

A good place to start:

git grep -i monotone git grep -i constraints

It looks to me like your new commit added new code but didn't remove this parameter-manipulation or update the tests. Please, do those things.

And if you are using LLMs to help you that's fine, but please review the code yourself and please write comments in these discussion threads yourself.

It looks to me like your new commit added new code but didn't remove this parameter-manipulation or update the tests. Please, do those things.

And if you are using LLMs to help you that's fine, but please review the code yourself and please write comments in these discussion threads yourself.

Thanks for the feedback. @jameslamb

I’m planning to:

Remove the override of monotone_constraints_method = "basic" in config.cpp

Skip creating and using constraints entirely in the tree learner when monotone_constraints is empty (using nullptr guards)

Update tests to reflect that monotone_constraints_method is preserved but ignored

Does this align with your expectation of avoiding all constraints-related code paths?

yes

Thanks for the feedback @jameslamb .

I’ve updated the PR to avoid mutating monotone_constraints_method and instead skip all monotone-constraint-related code paths when monotone_constraints is empty.

Specifically:

removed the fallback logic in config.cpp

ensured constraints are not initialized when not needed

added guards to avoid accessing constraints when they are not present

updated the test to check that the parameter is preserved and only ignored internally

Let me know if this approach aligns with what you had in mind, or if you'd like it handled differently.

yes

hey @jameslamb , just checking if any changes are needed from my side. or waiting to be reviewed.

jameslamb · 2026-03-19T03:58:18Z

Please do also run the linting / autoformatting locally before pushing commits.

pre-commit run --all-files

Docs on pre-commit if you are not familiar with it: https://pre-commit.com/#install

…onstraints Add test case in test_engine.py to verify that setting monotone_constraints_method without monotone_constraints: - emits a warning - does not crash training - falls back to 'basic' Follows existing test structure and dataset utilities.

Remove standalone test file and integrate test into existing test_engine.py following project conventions. Also add coverage for monotone_constraints_method without monotone_constraints to ensure warning is emitted and fallback to 'basic' occurs.

Shreyas-S-809 · 2026-03-19T05:24:10Z

Please do also run the linting / autoformatting locally before pushing commits.
pre-commit run --all-files
Docs on pre-commit if you are not familiar with it: https://pre-commit.com/#install

Thanks for the review, i have updated the PR, please take a look and let me know if any changes are required.

… is empty When monotone_constraints is empty, the constraints factory still created AdvancedLeafConstraints or IntermediateLeafConstraints based on monotone_constraints_method. These implementations assume valid constraint data and can lead to invalid memory access. This change updates LeafConstraintsBase::Create() to return BasicLeafConstraints when no monotone constraints are provided, effectively disabling constraint-specific logic without modifying user parameters. This avoids unintended side effects from mutating configuration while ensuring safe execution paths.

…o fix6841

Ensure that monotone constraints logic is completely bypassed when monotone_constraints is not provided, instead of mutating monotone_constraints_method or falling back to 'basic'. Changes: - Avoid initializing constraint structures when no monotone constraints are set in serial_tree_learner.cpp - Guard all constraint-related calls to prevent null dereferencing - Remove parameter mutation logic from config.cpp that overwrote monotone_constraints_method - Ensure monotone_constraints_method is preserved as provided by the user - Update tests in est_engine.py to verify: - warnings are emitted - parameter is preserved - training completes without errors This approach eliminates unintended side effects and ensures that constraint-specific logic is only executed when explicitly requested.

fix: prevent crash when monotone_constraints_method is set without mo…

5b36cb9

…notone_constraints

Shreyas-S-809 requested review from StrikerRUS, borchero, guolinke, jameslamb, jmoralez and shiyu1994 as code owners March 16, 2026 16:21

Shreyas-S-809 mentioned this pull request Mar 16, 2026

Training crashes when "monotone_constraints_method" is set to "advanced" #6841

Open

jameslamb added the fix label Mar 19, 2026

jameslamb requested changes Mar 19, 2026

View reviewed changes

jameslamb added the in progress label Mar 19, 2026

Shreyas-S-809 added 2 commits March 19, 2026 10:34

Shreyas-S-809 requested a review from jameslamb March 19, 2026 05:17

Shreyas-S-809 and others added 4 commits March 30, 2026 10:07

Merge branch 'master' of https://github.com/lightgbm-org/LightGBM int…

876c5a8

…o fix6841

Merge branch 'master' into fix6841

b1548a8

Conversation

Shreyas-S-809 commented Mar 16, 2026

Summary

Fix

Tests

Example

Uh oh!

jameslamb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Shreyas-S-809 Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Shreyas-S-809 Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Shreyas-S-809 Apr 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jameslamb commented Mar 19, 2026

Uh oh!

Shreyas-S-809 commented Mar 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Shreyas-S-809 Mar 30, 2026 •

edited

Loading

Shreyas-S-809 Mar 30, 2026 •

edited

Loading

Shreyas-S-809 Apr 5, 2026 •

edited

Loading