qrandint/qlograndint's documented inclusive upper bound is never sampled
- 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