Feat/feasibility check - #291
Conversation
- proposal: introduce 3-phase framing (resource/data/config), add resource-phase refinements (warm cache, n_jobs × VRAM, refit_after, Hub reachability, CatBoost GPU sanity), data-quality phase (token truncation, split readiness, partial descriptions, embedder dim), config sanity phase, updated example output, CLI surface, out-of- scope deferrals - _advisor package: hardware detection (CUDA/MPS/CPU with broken-CUDA fallback), HF Hub metadata + warm-cache probe + offline heuristics, three-phase run_preflight returning structured PreflightReport, text + JSON renderers - autointent-advisor CLI: inspect <preset|config> and recommend subcommands; placeholder dataset stats when no --dataset given - 88 offline tests covering hardware fallbacks, every bundled preset, severity routing, report serialization, name-pattern heuristics, AMP invariant, dump_modules / refit_after, CLI flows Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
| SearchSpacePreset = Literal[ | ||
| "classic-heavy", | ||
| "classic-light", | ||
| "classic-medium", | ||
| "nn-heavy", | ||
| "nn-medium", | ||
| "transformers-heavy", | ||
| "transformers-light", | ||
| "transformers-no-hpo", | ||
| "nn-heavy", | ||
| "zero-shot-llm", | ||
| "nn-medium", | ||
| "classic-heavy", | ||
| "transformers-no-hpo", | ||
| "classic-medium", | ||
| "zero-shot-encoders", | ||
| "classic-light", | ||
| ] | ||
| """Some presets that our library supports.""" | ||
| """Bundled search-space presets, listed in descending quality order. | ||
|
|
||
| The order is consumed by ``autointent._advisor.recommend`` to pick the | ||
| highest-quality feasible preset (lower index = higher quality).""" |
There was a problem hiding this comment.
это к сожалению неправда - выстроить пресеты в какой-то один порядок нельзя потому что под разные задачи нужны разные пресеты
например transformers-heavy будет ужасно работать если выборка маленькая
There was a problem hiding this comment.
Тут скорее по времени выполнения
There was a problem hiding this comment.
тогда надо докстринг изменить, сейчас он вводит в заблуждение читателей документации
| if mixed_precision: | ||
| bytes_per_sample //= 2 |
There was a problem hiding this comment.
тут не совсем такая формула, там же куча мастер копий еще - мне кажется стоит уточнить этот момент
такое сокращение может быть, но это слишком оптимистичная оценка - а мы хотим оценить затраты сверху а не снизу
There was a problem hiding this comment.
немного странно что цпу никак не влияет на оценки времени
voorhs
left a comment
There was a problem hiding this comment.
в целом по методике и алгоритам ок за исключением мелочей которые прокомментил (посмотрел не прямо все но пока это стоит исправить)
есть два пожелания:
- наверное стоит добавить какой-то обоснованности всем используемым формулам (ссылки на внешние ресурсы, бенчмарки, статьи в которых исследуется такое) - вообще с этого стоило начать выполнение этой задачи)
- очень неудобно ревьюить когда в одном бульоне приватные утилиты и публичные функции, мне кажется стоит руками самому как-то разнести все это на подфайлы и подпапки, потому что иишке это ок, а человечески очень тяжело когда файл на 800 строк и в нем центральный публичный метод с главным алгоритмом спрятан где-то посередине или в конце
# Conflicts: # pyproject.toml
| # rare class x linear-CV (LogisticRegressionCV cv=3 needs >=3 samples/class; | ||
| # multilabel path uses one-vs-rest without CV so the failure can't occur there) | ||
| has_linear = any(e.get("module_name") == "linear" for _, e in _walk_modules(search_space)) | ||
| if has_linear and stats.rare_classes and not stats.multilabel: | ||
| report.add( | ||
| "data", | ||
| Severity.OVER, | ||
| f"LogisticRegressionCV (cv=3) will fail: classes {stats.rare_classes[:5]} have <3 samples.", | ||
| ) |
There was a problem hiding this comment.
я не знаю есть ли тут баг, но на всякий случай перепроверь пожалуйста что эта проверка использует нужные сплиты (train/val/test) и не противоречит check_split_readiness в подмодуле data_handler
There was a problem hiding this comment.
а еще тут полагается что cv=3, но это же не всегда так
voorhs
left a comment
There was a problem hiding this comment.
норм! концептуально и по коду лучше, теперь это надо встроить в Pipeline.fit()
а еще надо провести эксперимент который оценивает насколько хорошо эта штука работает. у меня в голове такой дизайн: берешь как можно больше разных машин (нашу виртуалку, узел нашего кластера, свой ноутбук, свой другой ноутбук, свой пк), берешь разные серч спейсы и 2-3 датасета - и смотришь фолз/тру позитивы, фолз/тру негативы
если оформишь код в AutoIntent-experiments, то я на своих машинах тоже запущу и добавлю тебе кейсов в отчет
|
еще кстати есть такой вопрос: текущая реализация как-то учитывает кеширование эмбедингов? я могу потом как нибудь у клодика спросить, написал сюда чтобы не забыть (ну или может ты ответишь) |
Advisor (src/autointent/_advisor/): - fix linear/classic time formula and transformer VRAM under-prediction (34 B/token/layer upper bound; batch-scaled) - device-class per-step transformer time lookup - optional embedding-cache warmth probe: predict 0 forward + 0 disk_embedding_cache when caller certifies warmth - stop silent-zero estimates for cnn/rnn/sklearn (emit not-estimated row) - count cross-encoder / reranker downloads via cross_encoder_config + transformer_config fallback - conservative + loud low-confidence fallback (large-model defaults; TIGHT finding instead of buried note) - new reduce_to_fit + ReduceToFitError workflow with empty-scoring guard Calibrator (scripts/calibrate_advisor.py + run_calibration_banking77.sh): - fix CUDA VRAM measurement (per-module reset was clobbering peak); ratios computed at serialization time - --clear-embedding-cache + cache-policy tagging - --budget-vram-gb, --require-cuda, --subsample-per-class, --repeats, --dataset nargs="+" for constrained-hardware and shape sweeps - incremental atomic per-preset JSON checkpoint - role classification (embedder/scorer/decision) + time_by_role_s - optional-extras skip (peft/catboost/openai) — clean skip row instead of fit-failed - --presets accepts .yaml paths; coverage_preset.yaml packs lora, ptuning, dnnc, gcn, description_cross for module coverage - in-process CLI smoke (autointent-advisor inspect --json) compared to direct-API report every preset - per-step timing captured via monkey-patched HF Trainer callback → step_timings on each module record Tests: 111 passing (test_reduce_to_fit + test_calibration_tracker new). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
HF's CallbackHandler.call_event dispatches with a bare ``getattr(callback, event)(...)`` — no hasattr probe — so a plain class that only implements on_step_begin/on_step_end crashes with ``AttributeError: '_StepTimingCallback' object has no attribute 'on_train_begin'`` the moment a real bert trial runs through the calibrator's step-timing patch. Fix by subclassing ``transformers.TrainerCallback`` directly: every ``on_*`` hook is inherited as a proper no-op, so we only override the two we time. Lazy try/except on the import keeps the module loadable in classic-only environments — the fallback base is only used for the class definition (the callback is never instantiated there because ``_patch_trainer_for_step_timing`` bails out in the same ImportError branch). Regression test in test_calibration_tracker.py pins the isinstance contract. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The harness is a validation instrument, not library code. It now lives in Darinochka/AutoIntent-experiments PR #40 beside the results it produced. - drops the tests/ -> scripts/ import that broke mypy (scripts/ is not a package) - reverts ruff-format-only churn in tests/test_deps.py, tests/ci/test_compute_matrix.py - gitignores local validation artifacts and Superpowers process docs
…ring compute-feasibility-advisor-proposal.md was removed in the harness-relocation commit but the advisor package's module docstring still pointed to it. Strip the sentence rather than repoint it — a later task rewrites this docstring wholesale.
Pipeline.fit's docstring already pointed users at autointent._advisor.run_preflight, so the only usable entry point was behind a leading underscore. Promote the package, keep internals private (runner.py -> _runner.py, workflows.py -> _workflows.py), and rename the console script to match the prog= the CLI already reports.
- inspect -> estimate (the old name shadowed the stdlib inspect module) - stats_from_dataset_obj -> dataset_stats - drop BUNDLED_PRESETS, load_config, stats_from_dataset from __all__ (CLI plumbing) - move PreflightError into advisor/_errors.py so there is one import path for it - mark the package experimental in its docstring - lock the surface with tests/advisor/test_public_surface.py
Finding.metric holds short names ('vram', 'ram', 'disk', 'time') but
_pick_module_to_drop tested membership against ['vram_gb', 'time_hours',
'ram_gb', 'disk_download_gb']. The sets are disjoint, so the lookup never
matched and driver_key always fell through to 'vram_gb' -- correct by accident
on VRAM-bound machines, wrong everywhere else.
Map the two namespaces explicitly and drop disk from the priority walk, since
driver rows carry no per-module disk figure (documented as a VRAM proxy).
Every existing test used a _profile() with hardcoded ram_gb/free_disk_gb, which
is why a green suite missed this; the helper now parameterizes both.
Found during validation: Darinochka/AutoIntent-experiments#40, finding 3.
The branch reordered the public SearchSpacePreset literal into a cost ranking and derived cost_rank from get_args() declaration order, making a public type alias's element order load-bearing. Restore dev's ordering and move the ranking into PRESET_COST_ORDER, covered by a test so adding a preset fails loudly instead of silently sorting it cheapest-last.
28 ruff findings (UP035, F401, RUF002/003, D205/D209, N806, PLR2004, EM101/EM102/TRY003) and 6 mypy errors. No behaviour change. The ModelMeta fix is a real latent bug: _fold_disk_costs reused the name 'meta' for both a ModelMeta loop variable and a ModelMeta | None lookup, which is why mypy also reported the None guard as unreachable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
_resource_phase took 12 keyword arguments and ran to 59 statements / 17 branches / complexity 19; _apply_embedding_cache hit complexity 12. Bundle the config-shaped inputs into a frozen _ResourceInputs and extract the two estimation passes plus the embedding-cache first-pay bookkeeping. Pure restructuring -- verified byte-identical estimates across all 10 bundled presets before and after. No noqa suppressions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
preflight defaulted to 'warn', so every fit() ran the advisor -- and resolve_model() calls HfApi().model_info() unconditionally per distinct model name, with no cache-first short-circuit. That put N Hub round-trips and, when offline, one WARNING per model on the library's hottest path, for estimates that are explicitly heuristic. Default to 'off'; all three modes still work. Also make the advisor import lazy so 'import autointent' never pulls in huggingface_hub probes, and rewrite the preflight tests: they previously ran a real classic-light fit inside 'except Exception: pass', so they passed even when the fit failed for unrelated reasons.
Renaming _advisor to advisor makes autoapi publish the package, so the public docstrings are now user-facing. Add a prose page covering the CLI, reading a report, the Python API, and the Pipeline.fit gate. Documents the accuracy limits measured in Darinochka/AutoIntent-experiments#40 and softens the 'pessimistic upper bound' claims -- one preset measured 1.22x its predicted VRAM, so that guarantee doesn't hold. Written as docs/source/advisor.rst rather than a user_guides page: those are jupytext-executed during the docs build and would have to run real fits. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- price the filtered search space: run validate_modules before the preflight gate, so preflight no longer charges for modules fit() will discard (mlknn on multiclass, dnnc on multilabel and its ~6.4 GB reranker) - correct reduce_to_fit's public docstring, which still advertised the pruning order this branch fixed - document Severity, HardwareProfile and four published members; Severity's API page was rendering str.__doc__ - rewrite dataset_stats' docstring, which referenced a non-public name - add the console-script name regression test and correct test_hardware_detection.py's "no psutil" claim (both spec section F) - comment the second lazy-import site so it is not tidied back to module scope - rename _charge_first_forward to _charge_first_forward_if_classic and fix three stale sentences in _resource.py; no arithmetic touched Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
What this adds
A pre-flight compute feasibility advisor. Before any download or training, it estimates VRAM, RAM, disk, and wall-time for a search space on the current machine, and either reports or blocks when the budget will not hold.
Two entry points:
autointent-advisor inspect <preset|config.yaml>andautointent-advisor recommend, both accepting a real--datasetor--n-samples/--n-classes/--avg-tokensplaceholders. Non-zero exit when nothing is feasible, so it works as a CI gate.autointent.advisor, a deliberately narrow 15-name surface marked experimental.Docs:
docs/source/advisor.rst. The rename from_advisortoadvisoralso means the package now publishes an auto-generated API reference page.Validation
Validated on real constrained hardware (RTX 3060 Laptop, 6 GB VRAM / 16 GB RAM) in Darinochka/AutoIntent-experiments#40, which closes issue #39 there: 4/4 fitted presets matched their predicted verdict — both OVER predictions actually OOM'd, both feasible predictions actually fit, reduce-to-fit produced a runnable pipeline, and the strict gate aborted before allocating any VRAM.
That validation also surfaced a bug now fixed here (below), and two accuracy limits that are documented rather than fixed: VRAM is close but not a guaranteed upper bound (one preset at 1.22× prediction), and wall-time estimates are indicative only, with measured error running in both directions. Both are stated plainly in
docs/source/advisor.rst; the "pessimistic upper bound" language has been removed from the code, since the measurements do not support it.Reviewer notes
Pipeline.fit(preflight=...)defaults to"off".resolve_model()callsHfApi().model_info()per distinct model name with no cache-first short-circuit, so a default-on gate would add N Hub round-trips to everyfit()and one WARNING per model when offline — on the library's hottest path, for estimates that are explicitly heuristic. The advisor import is lazy and a test assertsimport autointentdoes not importautointent.advisor. Flipping the default to"warn"later is purely additive.psutilbecomes a new core dependency. There is no cross-platform stdlib RAM query, and no psutil-absent fallback exists —_hardware.pyimports it unguarded. It is small and ubiquitous, but it is a genuine addition to the base install, so flagging it explicitly rather than burying it.Bug fix:
reduce_to_fitpruned by VRAM regardless of the binding constraint.Finding.metricholds short names ("vram","ram","disk","time") but_pick_module_to_droptested membership against["vram_gb", "time_hours", ...]. Disjoint sets, so the lookup never matched and every prune silently fell back to VRAM — correct by accident on VRAM-bound machines, wrong everywhere else. Every existing test used a profile with hardcodedram_gb/free_disk_gb, which is why a green suite missed it. Found in experiments#40, finding 3.Behaviour change:
validate_modulesnow runs before the preflight gate. Previously preflight priced the unfiltered search space, so it charged for modulesfit()was about to discard —mlknnon multiclass datasets, anddnnc(plus its ~6.4 GB reranker) on multilabel. On a small GPU that could flip the verdict to OVER and raisePreflightErroron a fit that would have succeeded.SearchSpacePresetkeeps its original ordering. An earlier revision of this branch reordered the publicLiteralinto a cost ranking and then derived the ranking fromget_args()declaration order, making a public type alias's element order load-bearing. That is now an explicitPRESET_COST_ORDERwith a test that fails if a preset is added to one and not the other.PreflightErroris importable only fromautointent.advisor. There is deliberately no re-export fromautointent._pipeline:autointent/__init__.pyimports._pipeline, so an eager advisor import there would load the advisor on everyimport autointentand defeat the lazy-import guarantee above.The calibration harness is not in this diff.
scripts/calibrate_advisor.pyand friends were validation instruments; they belong beside the results they produced, not in the library — and one of them was imported by a test fromscripts/, which is not a package, breaking mypy. See the companion PR against the experiments repo.Verification
ruff checkclean ·mypy src/autointent testsclean (323 files) · docsmypyclean · 143 advisor + preflight tests passing ·make docssucceeds · no JSON-schema diff ·import autointentdoes not importautointent.advisor·advisor.__all__is exactly 15 names.Please apply the
full-cilabel before merging — the full matrix and the docs build have never run on this branch, and the advisor has platform-specific code (psutil,torch.mps,shutil.disk_usage, Windows drive-letter parsing).