镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

Forward free_mem_ratio from compute_estimator to the evaluation - #1591

Merged
Li Jiang (thinkall) merged 5 commits into
microsoft:mainfrom
VenishPaneliya:forward-free-mem-ratio
Sep 8, 2026
Merged

Li Jiang (thinkall) merged 5 commits into
microsoft:mainfrom
VenishPaneliya:forward-free-mem-ratio

Conversation

@VenishPaneliya

Copy link
Copy Markdown
Contributor

Why are these changes needed?

free_mem_ratio is a documented AutoML.fit setting:

free_mem_ratio: float between 0 and 1, default=0. The free memory ratio to keep during training.

AutoMLState._compute_with_config_base does pass state.free_mem_ratio into compute_estimator, but compute_estimator then hands a hard-coded 0 to the evaluation in both branches:

        val_loss, metric_for_logging, train_time, pred_time = task.get_val_loss(
            ...
            free_mem_ratio=0,          # holdout path
        )
    else:
        val_loss, metric_for_logging, train_time, pred_time = task.evaluate_model_CV(
            ...
            free_mem_ratio=0,          # CV path
        )

So the setting is discarded for every trial of the search. It survives only in train_estimator, which forwards it correctly (free_mem_ratio=free_mem_ratio), meaning the final retrain honours the value while the search — where the repeated model fits happen and memory pressure is highest — silently runs at 0.

The value is not cosmetic; it sizes the training memory limit in BaseEstimator.fit:

mem.available * (1 - free_mem_ratio) + psutil.Process(os.getpid()).memory_info().rss

so someone setting free_mem_ratio=0.25 to keep headroom during an AutoML run does not get it where it matters.

Evidence

Spying on GenericTask.evaluate_model_CV during a small AutoML.fit(..., free_mem_ratio=0.25, eval_method="cv"):

free_mem_ratio received per call
before 0
after 0.25

Only the two call sites inside compute_estimator are changed; the free_mem_ratio=0 defaults in the three function signatures are left as they are.

Related issue number

None — found while reading the search path, not from a filed issue.

Checks

  • I've used pre-commit to lint the changes in this PR — ruff check and ruff format --check are clean on both files.
  • I've included any doc changes needed — not needed; the existing documentation already describes the intended behaviour, this makes the code match it.
  • I've added tests corresponding to the changes introduced in this PR — test/automl/test_free_mem_ratio.py records what evaluate_model_CV receives during a real AutoML.fit. It fails on main with assert {0} == {0.25} and passes with this change.
  • I've made sure all auto checks have passed.

`compute_estimator` takes `free_mem_ratio` and is given the user's value
by `AutoMLState._compute_with_config_base`, but both branches pass a
hard-coded 0 on to the evaluation:

    free_mem_ratio=0,   # holdout path
    free_mem_ratio=0,   # CV path

so the setting is dropped for every trial of the search. It survives only
in `train_estimator`, which forwards it correctly, so the final retrain
honours it while the search - where the repeated fits happen and memory
pressure is highest - does not.

The value reaches `estimator.fit`, which uses it to size the training
memory limit:

    mem.available * (1 - free_mem_ratio) + rss

Spying on `GenericTask.evaluate_model_CV` during a small `AutoML.fit`
with `free_mem_ratio=0.25`:

    before: every call receives 0
    after:  every call receives 0.25
@VenishPaneliya

Copy link
Copy Markdown
Contributor Author

@microsoft-github-policy-service agree

@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.

Reviewed the updated head: no blocking correctness findings. Both compute_estimator evaluation branches now forward the configured free_mem_ratio, consistent with final training; the default remains unchanged. I also confirmed the value reaches estimator fitting through both holdout and CV. I updated the branch and approved its pending workflow runs. The refreshed build matrix is still running, so I am leaving final approval pending CI rather than marking it merge-ready yet.

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.

🟡 Changes recommended

The holdout forwarding path lacks regression coverage.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Forwards free_mem_ratio into per-trial AutoML evaluation so memory headroom applies during search.

Changes:

  • Passes the configured ratio through holdout and cross-validation paths.
  • Adds cross-validation regression coverage.
File summaries
File Description
flaml/automl/ml.py Forwards the ratio to both evaluation paths.
test/automl/test_free_mem_ratio.py Verifies forwarding during CV search.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread test/automl/test_free_mem_ratio.py Resolved
The existing test only exercised the CV branch of `compute_estimator`.
The two branches call different helpers - `evaluate_model_CV` and
`get_val_loss` - so each needs its own coverage.

Adds the holdout equivalent, recording what `get_val_loss` receives
during an `eval_method="holdout"` search. Both tests fail against the
hard-coded 0 with `assert {0} == {0.25}`.
@VenishPaneliya

Copy link
Copy Markdown
Contributor Author

Thanks both, and thanks Li Jiang (@thinkall) for merging main in and approving the workflow runs.

copilot-pull-request-reviewer is right that the holdout path had no coverage — the two branches of compute_estimator call different helpers (evaluate_model_CV vs get_val_loss), so one test didn't exercise the other. Added test_free_mem_ratio_reaches_the_search_holdout, which records what get_val_loss receives during an eval_method="holdout" search.

Both tests now fail against the hard-coded 0 with assert {0} == {0.25} and pass with the change. I rebased onto your updated branch rather than force-pushing, and re-ran them on top of the merged main: 2 passed. ruff check is clean (it wanted the new import sorted, which is fixed).

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 focused fix preserves existing defaults and covers both affected branches with regression tests.

Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@thinkall
Li Jiang (thinkall) added this pull request to the merge queue Sep 8, 2026
Merged via the queue into microsoft:main with commit 2a55656 Sep 8, 2026
13 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.

3 participants