Repository navigation
New strategy to deal with monotonic constraints with dropped columns - #1025
Open
Marc-Antoine Schmidt (MarcAntoineSchmidtQC) wants to merge 4 commits into
Open
Marc-Antoine Schmidt (MarcAntoineSchmidtQC) wants to merge 4 commits into
Marc-Antoine Schmidt (MarcAntoineSchmidtQC) wants to merge 4 commits into
Conversation
Marc-Antoine Schmidt (MarcAntoineSchmidtQC)
requested review from
Jan Tilly (jtilly) and
Martin Stancsics (stanmart)
as code owners
October 30, 2026 16:27
Marc-Antoine Schmidt (MarcAntoineSchmidtQC)
requested review from
Matthias Schmidtblaicher (MatthiasSchmidtblaicherQC)
and
a balanced review from Copilot
and removed request for
Jan Tilly (jtilly) and
Martin Stancsics (stanmart)
October 30, 2026 16:27
Copilot started reviewing on behalf of
Marc-Antoine Schmidt (MarcAntoineSchmidtQC)
October 30, 2026 16:28
Marc-Antoine Schmidt (MarcAntoineSchmidtQC)
requested review from
Jan Tilly (jtilly) and
Martin Stancsics (stanmart)
October 30, 2026 16:28
1 task done
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Critical constraint-handling issues can exclude valid categorical-interaction fits and silently leave spline monotonicity unenforced on supported tabmat versions lacking the required metadata.
2 open issues
What changed in this PR
This PR addresses #1021 by accounting for implicitly zero coefficients when constructing monotonic constraints for dropped spline bases and categorical levels. Spline detection relies on metadata introduced by tabmat PR #550.
Changes:
- Track dropped-column metadata and add sign constraints for the first remaining coefficient.
- Pass that metadata through the estimator’s constraint setup.
- Add prediction-level regression tests and document the fix and dependency caveat.
| File | Description |
|---|---|
tests/glm/test_formula.py |
Tests monotone predictions with dropped columns and preserves unconstrained offsets for full spline bases. |
src/glum/_glm.py |
Passes dropped-column metadata into constraint construction. |
src/glum/_formula.py |
Detects dropped columns and anchors the first remaining coefficient. |
CHANGELOG.rst |
Describes the fix and required tabmat metadata. |
Review effort: Balanced
Comment on lines
+197
to
+199
| _, state = model_spec.encoder_state.get(expr, (None, {})) | ||
| column_state: dict[str, Any] = next(iter(state.values()), {}) | ||
| return column_state.get("spans_intercept") is False |
Matthias Schmidtblaicher (MatthiasSchmidtblaicherQC)
approved these changes
Oct 30, 2026
Matthias Schmidtblaicher (MatthiasSchmidtblaicherQC)
left a comment
Matthias Schmidtblaicher (MatthiasSchmidtblaicherQC)
left a comment
Contributor
There was a problem hiding this comment.
Leveraging formulaic's spans_intercept here instead of reimplementing the parsing makes total sense, thanks for the implementation.
| dropped_first: set[tuple[str, str]] = set() | ||
|
|
||
| for _term, scoped_terms, column_names in model_spec.structure: | ||
| # A term like C(a):C(b) without main effects splits into several scoped |
There was a problem hiding this comment.
I don't really get this comment.
Member
Author
There was a problem hiding this comment.
Fair, let me reword it.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Paired with tabmat's pr 550, this PR fixes issue #1021. It uses a more direct approach compared to #1023.
Checklist
CHANGELOG.rstentry