Skip to content

Add explicit support for certain objectives and apply the sigmoid when appropriate#118

Merged
ndawe merged 2 commits into
maxmind:mainfrom
ndawe:ndawe/base-score-and-regression
Jun 15, 2026
Merged

Add explicit support for certain objectives and apply the sigmoid when appropriate#118
ndawe merged 2 commits into
maxmind:mainfrom
ndawe:ndawe/base-score-and-regression

Conversation

@ndawe

@ndawe ndawe commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

This also fixes a lingering base_score issue. This hasn't been a problem with logistic objectives when base_score is 0.5 as that reduces to a baseline raw margin of 0. But if base_score is anything else, or if the objective is something like reg:squarederror then xgb2code generated code will produce incorrect output.

Summary by CodeRabbit

  • Tests
    • Expanded automated test coverage with a new case validating generation and execution for the "reg-squarederror" model. This exercises the end-to-end code-generation and run workflow for an additional model, increasing confidence in model handling and reducing regression risk.

@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR adds one entry for the "reg-squarederror" model to the TestGenerateAndRunModels table-driven test so the generation and go-run execution flow runs for that model.

Changes

Test coverage for reg-squarederror model

Layer / File(s) Summary
Add reg-squarederror test case
main_test.go
A new test entry for the "reg-squarederror" model is added to the TestGenerateAndRunModels table, ensuring code generation and execution are exercised for that model.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Poem

🐰 A small test hops into the row,
For squarederror it learns to show,
Generation runs and go will try,
A tiny case beneath the sky,
CI nods, the tests say hi!

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The PR title addresses the main objectives (adding regression support and sigmoid handling) but the actual changeset only adds a test case for reg:squarederror, not the core implementation changes described in the objectives. Verify whether this PR contains only the test additions or if the main implementation changes are also included. If only tests are present, consider a more specific title like 'Add test for reg:squarederror model support' or combine with implementation commits.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces support for parsing XGBoost model metadata, specifically handling objectives like binary:logistic, reg:logistic, and reg:squarederror, and incorporates the model's base score (intercept) and objective function into the generated Go code. It also adds regression test data and corresponding tests. The review feedback highlights a potential issue where NaN or Inf values in the base_score could bypass validation and cause silent failures or uncompilable code, suggesting explicit checks and unit tests for these edge cases. Additionally, it recommends setting random_state in the test model generation script to ensure reproducibility.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread gen/parse.go
Comment thread gen/parse_test.go
Comment thread gen/testdata/reg-squarederror/generate-model.py
@ndawe ndawe force-pushed the ndawe/base-score-and-regression branch 8 times, most recently from 52d982b to 41ebce1 Compare June 12, 2026 14:50
@ndawe ndawe changed the title Add explicit support for certain regression objectives and apply the sigmoid when appropriate Add explicit support for certain objectives and apply the sigmoid when appropriate Jun 12, 2026
… of base_score and when a sigmoid should be applied
@ndawe ndawe force-pushed the ndawe/base-score-and-regression branch from 41ebce1 to 3852d56 Compare June 12, 2026 15:52

@horgh horgh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Just a couple minor comments.

Comment thread CHANGELOG.md Outdated
Comment thread CHANGELOG.md
@ndawe ndawe requested a review from horgh June 15, 2026 13:01
@ndawe ndawe merged commit 0f23d2b into maxmind:main Jun 15, 2026
14 of 15 checks passed
@ndawe ndawe deleted the ndawe/base-score-and-regression branch June 15, 2026 18:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants