Add native step_size support to RangeParameter (#5213)#5213
Open
saitcakmak wants to merge 1 commit into
Open
Conversation
|
@saitcakmak has exported this pull request. If you are a Meta employee, you can view the originating Diff in D107274057. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5213 +/- ##
========================================
Coverage 96.50% 96.51%
========================================
Files 617 617
Lines 69776 69889 +113
========================================
+ Hits 67339 67452 +113
Misses 2437 2437 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary: Adds a `step_size` arg to `RangeParameter` that snaps values to a grid anchored at `lower` (in `cast()`), for both FLOAT and INT parameters. This is the first diff in the step_size unification stack: `step_size` will subsume both the discrete-grid and limited-resolution (`digits`) use cases under one knob. - Next diff will add storage support. The internal DB has already been updated to include the new column. - We will then migrate all current usage off `digits` and onto `step_size`. - We will add support for treating low-cardinality float-range parameters as discrete in `Adapter`, so that it is efficiently optimized over the correct grid (rather than having to use continuous optimization + rounding). - At this point, we will have proper support for `step_size`, so we can update the ax/api usage to leverage it, rather than resolving to `ChoiceParameter`. - We can then deprecate `digits` and do any remaining clean-up. In this diff `step_size` coexists with the existing `digits` arg (they are mutually exclusive at construction). Subsequent diffs in the stack migrate storage (JSON + SQA), transforms and utils, and the public API (`RangeParameterConfig`) to `step_size`, then deprecate `digits` in favor of it. Behavior: - `cast()` rounds `(value - lower) / step_size` to the nearest integer and returns `lower + n * step_size`. It does NOT clamp to `[lower, upper]`: an out-of-bounds input (e.g. a historical observation recorded outside the current bounds) snaps to the nearest grid point, which may itself be out of bounds. This mirrors the non-`step_size` `cast()`, which leaves out-of-bounds values in place rather than silently moving them into range — range validity is enforced by `validate()`, not `cast()`. - Both bounds must lie on the grid: `(upper - lower)` must be an integer multiple of `step_size` (within `EPS`). Off-grid bounds are rejected at construction. This guarantees `upper` is itself a feasible value, so a value near the upper bound snaps to `upper` rather than to a grid point short of it. - `step_size` must be strictly positive, and must be integer-valued for INT parameters. - `cardinality()` accounts for `step_size`: a grid-valued FLOAT reports the finite number of grid points instead of `inf`, and a grid-valued INT counts grid points rather than every integer in `[lower, upper]`. `step_size` defines a discrete grid but does not, by itself, force discrete acquisition optimization; how the optimizer treats the parameter depends on the grid cardinality and is determined at the generator level. Differential Revision: D107274057
c439254 to
b575948
Compare
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.
Summary:
Adds a
step_sizearg toRangeParameterthat snaps values to a grid anchored atlower(incast()), for both FLOAT and INT parameters. This is the first diff in the step_size unification stack:step_sizewill subsume both the discrete-grid and limited-resolution (digits) use cases under one knob.digitsand ontostep_size.Adapter, so that it is efficiently optimized over the correct grid (rather than having to use continuous optimization + rounding).step_size, so we can update the ax/api usage to leverage it, rather than resolving toChoiceParameter.digitsand do any remaining clean-up.In this diff
step_sizecoexists with the existingdigitsarg (they are mutually exclusive at construction). Subsequent diffs in the stack migrate storage (JSON + SQA), transforms and utils, and the public API (RangeParameterConfig) tostep_size, then deprecatedigitsin favor of it.Behavior:
cast()rounds(value - lower) / step_sizeto the nearest integer and returnslower + n * step_size. It does NOT clamp to[lower, upper]: an out-of-bounds input (e.g. a historical observation recorded outside the current bounds) snaps to the nearest grid point, which may itself be out of bounds. This mirrors the non-step_sizecast(), which leaves out-of-bounds values in place rather than silently moving them into range — range validity is enforced byvalidate(), notcast().(upper - lower)must be an integer multiple ofstep_size(withinEPS). Off-grid bounds are rejected at construction. This guaranteesupperis itself a feasible value, so a value near the upper bound snaps toupperrather than to a grid point short of it.step_sizemust be strictly positive, and must be integer-valued for INT parameters.cardinality()accounts forstep_size: a grid-valued FLOAT reports the finite number of grid points instead ofinf, and a grid-valued INT counts grid points rather than every integer in[lower, upper].step_sizedefines a discrete grid but does not, by itself, force discrete acquisition optimization; how the optimizer treats the parameter depends on the grid cardinality and is determined at the generator level.Differential Revision: D107274057