Skip to content

Preserve per-row categories in batched kernel transforms - #626

Merged
t-muser merged 1 commit into
bayesian-optimization:masterfrom
AHMETHAKANBEZIR1:fix/categorical-kernel-row-indexing
Oct 2, 2026
Merged

t-muser merged 1 commit into
bayesian-optimization:masterfrom
AHMETHAKANBEZIR1:fix/categorical-kernel-row-indexing

Conversation

@AHMETHAKANBEZIR1

@AHMETHAKANBEZIR1 AHMETHAKANBEZIR1 commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #625

Use paired row/category indices when constructing categorical one-hot rows. The original indexing selects every category present in the batch for every row, collapsing distinct categories and making the kernel depend on other rows in the batch.

One regression checks mixed/repeated categories, square and rectangular wrapped RBF kernel values against direct one-hot RBF inputs, and a single-row control.

Validation (Windows CPU, Python 3.12):

  • Untouched master af8b928: new regression fails; identity category inputs transform to all ones, RBF covariance is all ones and a real GP fit to [0,1,2] predicts [1,1,1].
  • After fix the real GP predicts the distinct targets within 1e-7; kernel and gradient match the direct RBF reference.
  • Full collected suite: 176 passed, 23 warnings (NumPy 2.5.3, SciPy 1.18.1, sklearn 1.9.1).
  • Full parameter module: 11 passed with that environment and NumPy 1.26.4 / SciPy 1.16.3 / sklearn 1.7.2.
  • Full configured Ruff 0.12.3 pre-commit lint/format and git diff checks pass; final formatted regression rerun passes.

GPU, other Python versions, docs build and gallery notebooks were not run. The current notebook-test path collects no examples, so full collected tests do not validate the gallery.

AI assistance: Codex autonomously implemented the fix and ran validation; no independent human review has occurred. Codex is recorded as a commit coauthor.

Summary by CodeRabbit

  • Bug Fixes
    • Corrected categorical parameter handling for batched inputs, so each row is converted to a one-hot vector based on its own category scores.
    • Preserved expected behavior for single-row inputs and kernel covariance calculations.

@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: d32c7748-dbaa-4edc-bc6a-4963bcb87d68

📥 Commits

Reviewing files that changed from the base of the PR and between af8b928 and 16223e8.

📒 Files selected for processing (2)
  • bayes_opt/parameter.py
  • tests/test_parameter.py

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 5 remain after this review.


📝 Walkthrough

Walkthrough

CategoricalParameter.kernel_transform now assigns one-hot values independently for each row. Tests cover batch output, single-row shape, and wrapped RBF kernel covariance.

Changes

Categorical kernel transform

Layer / File(s) Summary
Row-wise categorical transform and tests
bayes_opt/parameter.py, tests/test_parameter.py
The transform sets each row’s argmax category using paired row and category indices. Tests check batch and single-row output shapes, plus wrapped RBF kernel results for self-covariance and cross-covariance inputs.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 16223

The change restores per-row categorical kernel inputs and adds coverage for the affected cases; no merge-blocking risk is evident.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the primary change: preserving each row's category during batched categorical kernel transforms.
Linked Issues check ✅ Passed The PR addresses the coding requirements in issue #625. CategoricalParameter.kernel_transform now uses paired row indices and per-row argmax category indices. The added test covers mixed and repeate…
Out of Scope Changes check ✅ Passed The changes are limited to the categorical kernel transform fix in bayes_opt/parameter.py and focused regression coverage in tests/test_parameter.py. Both changes directly support issue #625. No u…
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@codecov

codecov Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.36%. Comparing base (af8b928) to head (16223e8).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #626   +/-   ##
=======================================
  Coverage   98.36%   98.36%           
=======================================
  Files          10       10           
  Lines        1220     1220           
=======================================
  Hits         1200     1200           
  Misses         20       20           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@t-muser

t-muser commented Oct 2, 2026

Copy link
Copy Markdown
Member

Hey @AHMETHAKANBEZIR1,

thanks for this contribution, this bug does indeed seem critical. I'll take care of getting this merged.

As a more general thing (related to your other PRs #622 and #624), I maintain this repo in my free time, which is rapidly shrinking. The codebase here has been built by voluntary contributions reviewed by voluntary maintainers (currently only me). Historically, both the contributor and the reviewer essentially take responsibility for the code that gets merged. That only works if the effort is roughly balanced. When a contribution is generated and submitted autonomously by an agent, and, as your PR descriptions say, without any human review, all of that responsibility shifts onto me. These are small PRs so I can review them in little time, but the converse also holds, i.e. you could similarly review them without too much effort. For your other PRs I find it unlikely that they were triggered during a real use-case of this package, which gives the appearance of fully automated bug hunting/contribution farming.

For future contributions, I'd ask that you:

  • review and understand the changes yourself before opening a PR
  • explain why the issue matters in practice, e.g. whether you ran into it while using the library, not only that it can be triggered

Thanks for understanding.

@t-muser
t-muser merged commit e7ab639 into bayesian-optimization:master Oct 2, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Batched categorical kernel transform merges different categories

2 participants