Skip to content

Fix normalization of singleton integer search spaces - #1644

Open
林SO (Linxiushen) wants to merge 1 commit into
microsoft:mainfrom
Linxiushen:codex/fix-singleton-integer-normalization
Open

林SO (Linxiushen) wants to merge 1 commit into
microsoft:mainfrom
Linxiushen:codex/fix-singleton-integer-normalization

Conversation

@Linxiushen

Copy link
Copy Markdown
Contributor

Why are these changes needed?

A valid tune.randint(7, 8) contains only the integer 7. normalize() subtracts one from the exclusive upper bound, then divides by the resulting zero span. This makes tune.run() raise ZeroDivisionError before evaluating an objective when the singleton dimension is in low_cost_partial_config.

The corresponding lograndint(7, 8) produces a NaN normalized value, and FLOW2's integer log-space step calculation also divides by zero.

Map a singleton, unquantized integer domain to the center of the normalized interval and skip its nonexistent log-space step constraint. The regression tests exercise the actual offline HPO API alongside another varying parameter, and check ordinary and quantized integer endpoints.

Related issue number

None found in the issue/PR search. Reproduced on main at 12b0f15e4b2c00fe119d9224ed5333f8fc6bab34.

Validation

  • Unmodified base: 8 new regression cases fail with ZeroDivisionError / FloatingPointError; 4 endpoint controls pass.
  • Fixed: 49 tests pass across test_singleton_integer.py, test_space.py, test_sample.py, test_searcher.py, and test_zero_metric.py.
  • Windows CPU, Python 3.12.14, NumPy 1.26.4, Optuna 3.6.1. Offline arithmetic objectives; AutoML estimator training, Ray integration and the full suite were not run.

Checks

  • I've used pre-commit to lint the changes in this PR.
  • I've included any doc changes needed for https://microsoft.github.io/FLAML/. No documentation change is needed for this correction to an existing valid search space.
  • I've added tests corresponding to the changes introduced in this PR.
  • I've made sure all auto checks have passed.

AI assistance: implemented and tested with Codex.

Copilot AI 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.

🟢 Approval recommended

The narrow guards address both zero-span calculations, with regression coverage and no unresolved findings.

No open issues 🎉

What changed in this PR

Fixes singleton integer search spaces so FLAML tuning avoids zero-span division errors.

Changes:

  • Maps singleton, unquantized integers to the normalized midpoint.
  • Skips FLOW2’s log-space step constraint for fixed integers.
  • Adds normalization, tuning, and endpoint regression tests.
File Description
test/​tune/​test_singleton_integer.py Tests singleton domains and ordinary/quantized endpoints.
flaml/​tune/​space.py Handles singleton integer normalization.
flaml/​tune/​searcher/​flow2.py Skips nonexistent singleton log-space steps.

Review effort: Balanced

@thinkall Li Jiang (thinkall) left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Overall review at e2bf1d2814e75e59682599af1b0346665ea92554: fix verified; this is not an approval.

I reassessed the complete three-file PR and connected normalize/denormalize and FLOW2 step-bound code. Singleton unquantized integers map to the finite midpoint and all checked inverse positions return the sole legal value; the log-step continue skips no other state update. All 49 targeted tests pass, including ordinary/quantized endpoint controls. Independent exact-main functions reproduce the original division/floating-point failures at negative, zero, positive and log bounds; four current offline mixed-space HPO cases make six evaluations each with variation in the live parameter. Fixed-only spaces return one valid trial under a bounded test budget and the direct FLOW2 log-step bound is finite. No blocking code issue was found. Separate Ray/distributed/platform and full optimizer-backend matrices were not run.

The Low review-code harness is CLEAN (actual gpt-6.1-sol), with audited input/model/read independence. Canonical evidence was committed locally after full all-file pre-commit passed. No contributor implementation was edited or pushed, and no other reviewer's thread was changed.

Approval is withheld: applicable check contexts are not all successful; actual Build: in_progress/None.

Posted by thinkall-agent-auto-reviewer

@thinkall Li Jiang (thinkall) left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Overall review of the complete current PR at e2bf1d2814e75e59682599af1b0346665ea92554: approved.

I freshly reread all production/test changes and the connected FLOW2 step-bound loop and normalization/inverse formulas. Singleton unquantized integer bounds now map to a finite midpoint, the inverse retains the sole legal integer, and the fixed log-space constraint no longer divides by zero. The continue skips no unrelated state update; ordinary and quantized endpoint behavior remains intact. No blocking issue found.

Validated unchanged-head review-code evidence was reused (Low, actual gpt-6.1-sol), including the prior 49 passing focused tests, exact-main failure comparisons, real six-evaluation mixed HPO cases, fixed-only exhaustion and inverse-position checks. These are unchanged-source results, not new model/test executions. Separate Ray/distributed/platform and optimizer-backend matrices are not claimed.

Fresh same-head gates confirm all discussions resolved, an explicit Balanced Copilot approval recommendation, every applicable check successful and actual Build/docs/formatting workflows successfully completed. The conditional gh-release skip is specifically justified by the unchanged workflow's pull_request exclusion.

Posted by thinkall-agent-auto-reviewer

This branch has not been deployed

No deployments
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.

3 participants