microsoft / microsoft/FLAML

qrandint/qlograndint's documented inclusive upper bound is never sampled

Open
#1,605 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Jupyter Notebook
Stars
4.4k
Forks
565
Avg merge
5d 8m
Merged PRs (30d)
17

Description

### Describe the bug

`qrandint`/`qlograndint` document "lower is inclusive, upper is also inclusive (!)", but the upper bound is unreachable in the common case. `Quantized.sample()` has a q=1 fast path that skips quantization and calls the wrapped `Integer` sampler directly, which draws exclusive of `domain.upper` (same contract as plain `randint`). Since `qrandint`'s default is `q=1`, the documented-inclusive upper is silently never sampled.

This isn't just cosmetic: `flaml/automl/time_series/ts_model.py`'s built-in ARIMA and SARIMAX estimators define their p/d/q/P/D/Q search spaces with `tune.qrandint(lower=0, upper=X, q=1)` at 9 call sites, so the shipped time-series AutoML search never explores the top value of any of those hyperparameters.

For q>1 it's also unreliable, just less consistently: the general branch rounds a raw exclusive-upper draw, so whether the nominal upper bound is reached depends on the rounding fraction.

### Steps to reproduce

```python
from flaml.tune.sample import qrandint
import numpy as np

rs = np.random.RandomState(0)
vals = {qrandint(1, 10).sample(spec=None, random_state=rs) for _ in range(20000)}
print(max(vals)) # 9, never 10

rs = np.random.RandomState(0)
vals = {qrandint(0, 10, 2).sample(spec=None, random_state=rs) for _ in range(20000)}
print(max(vals)) # 8, never 10
```

### Expected Behavior

`qrandint(1, 10)` should sample 10 at least occasionally, matching its own docstring.

### Additional Information

FLAML version: current main (f9e087c). I have a fix ready (extends the sampled range by one quantization step for `Integer` domains, then clamps) and I'll open a PR against this issue.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in flaml.tune.sample with Quantized.sample() and reproduce the qrandint examples using the shown NumPy RandomState. Check the documented inclusive upper bound for q=1 and q>1, including the time-series call sites in flaml/automl/time_series/ts_model.py. Done means the upper value can be sampled and regression tests cover both cases.

Written by the indexing model from the issue text.

Assessment

Tech stack
numpy, python
Domain
machine-learning
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.