feat(afd): add AFD default CLI mode built on the v2 Task architecture #1323
feat(afd): add AFD default CLI mode built on the v2 Task architecture #1323gehujun wants to merge 3 commits into
Conversation
WalkthroughAdds AFD serving mode across inference modeling, Pareto and sweep APIs, Task configuration, CLI execution, reporting, hardware resolution, and comprehensive unit tests. ChangesAFD inference contracts and modeling
AFD sweep and configuration
CLI and reporting
Hardware and validation
Estimated code review effort: 5 (Critical) | ~120 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
src/aiconfigurator/sdk/pareto_analysis.py (1)
1098-1117: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated A-pool capacity logic vs
_derive_a_batch_size.This block re-derives
a_model/a_partition/analytical max batch size, duplicating most of_derive_a_batch_size(lines 737-786) minus the 32-floor. Consider factoring out a shared helper (e.g._build_a_pool_and_capacity(..., align_to=..., apply_floor=...)) that both the fixed-batch and auto-batch branches call, so future capacity-model changes don't need to be kept in sync in two places.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/aiconfigurator/sdk/pareto_analysis.py` around lines 1098 - 1117, The A-pool capacity calculation here duplicates the same model/partition/batch-size derivation already implemented in _derive_a_batch_size, so update this branch to reuse a shared helper instead of re-creating a_model, a_partition, and _analytical_max_batch_size inline. Factor the common logic into something like _build_a_pool_and_capacity and have both the fixed-batch and auto-batch paths call it, keeping only the branch-specific floor/align behavior in the caller.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/aiconfigurator/cli/main.py`:
- Around line 1502-1505: The AFD path in main() is bypassing the
already-resolved model_path and system_name values, so it can KeyError when only
prefill/decode keys are present. Update the AFD branch that builds the task to
use the resolved local variables from the earlier fallback logic instead of
re-indexing task_config directly, and keep the same fallback behavior for both
model_path and system_name.
- Around line 1556-1590: The AFD branch in main.py builds a fixed afd_kwargs
whitelist and then calls Task(**...), which silently drops YAML fields that
Task.from_yaml would reject. Update the AFD construction path to either include
the missing AFD-related keys such as nextn, nextn_accept_rates, and any
*_quant_mode fields, or add Task.from_yaml-style validation so unconsumed keys
raise instead of being ignored. Keep the behavior consistent with the existing
agg/disagg path and use the Task/Task.from_yaml and afd_kwargs logic in this
function to locate the fix.
- Line 1505: The serving_mode validation in main.py already accepts afd, but the
user-facing experiment examples and docs still only document agg and disagg.
Update the serving_mode entries in src/aiconfigurator/cli/example.yaml and the
experiment sections in docs/cli_user_guide.md and docs/advanced_tuning.md to
include afd alongside the existing modes so users see it as a valid option.
In `@src/aiconfigurator/sdk/task_v2.py`:
- Around line 1108-1119: The pinned AFD topology path in task_v2.py accepts
afd_tp_a without validating that it evenly divides gpus_per_node, which can lead
to silent truncation in _run_afd_single_point and incorrect worker/GPU counts.
Add a fast-fail validation immediately after
_lookup_num_gpus_per_node(self.system_name) resolves gpus_per_node and before
the pinned-topology early return, checking that self.afd_tp_a divides
gpus_per_node whenever afd_n_a_nodes and afd_n_f_nodes are also pinned. Raise a
clear ValueError from this validation so invalid pinned settings are rejected
before any profiling or single-point execution begins.
- Around line 1160-1177: The AFD combined-with-PD prefill search-space values
are being overwritten unconditionally in the prefill resolution block, so
user-specified YAML/CLI overrides are lost. Update the logic in the
combined-with-PD handling around the build_disagg_parallel_lists call so each
prefill_*_candidates field is only assigned when the existing attribute is None,
matching the guard used in the quant-mode propagation loop and the
override-preserving behavior in _fill_role_search.
In `@tests/unit/sdk/test_afd_pareto.py`:
- Around line 263-274: The pytest.raises match string in
test_prefill_candidate_overflow_errors uses unescaped regex metacharacters, so
Ruff flags it. Update the match argument in this test to treat the literal
config path from _enumerate_afd_prefill_options as plain text by escaping the
dots in afd_config.prefill_search.max_candidates, or otherwise make the pattern
a valid regex that matches the intended error message exactly.
---
Nitpick comments:
In `@src/aiconfigurator/sdk/pareto_analysis.py`:
- Around line 1098-1117: The A-pool capacity calculation here duplicates the
same model/partition/batch-size derivation already implemented in
_derive_a_batch_size, so update this branch to reuse a shared helper instead of
re-creating a_model, a_partition, and _analytical_max_batch_size inline. Factor
the common logic into something like _build_a_pool_and_capacity and have both
the fixed-batch and auto-batch paths call it, keeping only the branch-specific
floor/align behavior in the caller.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 48f57bc6-c92c-489f-8779-95f0dbe84823
📒 Files selected for processing (17)
.gitignoresrc/aiconfigurator/cli/main.pysrc/aiconfigurator/cli/report_and_save.pysrc/aiconfigurator/cli/utils.pysrc/aiconfigurator/generator/facts/hardware.yamlsrc/aiconfigurator/generator/facts/request_resolution.pysrc/aiconfigurator/sdk/common.pysrc/aiconfigurator/sdk/inference_session.pysrc/aiconfigurator/sdk/pareto_analysis.pysrc/aiconfigurator/sdk/picking.pysrc/aiconfigurator/sdk/sweep.pysrc/aiconfigurator/sdk/task_v2.pytests/unit/cli/test_afd_default_cli.pytests/unit/cli/test_cli_workflow.pytests/unit/sdk/test_afd_parallel_lists.pytests/unit/sdk/test_afd_pareto.pytests/unit/sdk/test_afd_pareto_review_actions.py
📜 Review details
⏰ Context from checks skipped due to timeout. (5)
- GitHub Check: Cargo Deny
- GitHub Check: Build and Test (unit)
- GitHub Check: Rust/Python engine-step parity
- GitHub Check: Build and Test (e2e)
- GitHub Check: AIC Accuracy Regression Testing
⚠️ CI failures not shown inline (2)
GitHub Actions: Copyright Checks / copyright-checks: feat(afd): add AFD default CLI mode built on the v2 Task architecture
Conclusion: failure
erf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/trtllm/1.3.0rc10/scale_matrix_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/trtllm/1.3.0rc10/wideep_moe_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/trtllm/1.3.0rc15/SHARED_LAYER_REUSE.txt
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/vllm/0.19.0/context_attention_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/vllm/0.19.0/custom_allreduce_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/vllm/0.19.0/dsa_context_module_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/vllm/0.19.0/dsa_generation_module_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/vllm/0.19.0/encoder_attention_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/vllm/0.19.0/gdn_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/vllm/0.19.0/gemm_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/vllm/0.19.0/generation_attention_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/vllm/0.19.0/mla_context_module_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/vllm/0.19.0/mla_generation_module_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/vllm/0.19.0/moe_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/vllm/0.22.0/SHARED_LAYER_REUSE.txt
[WARN] Unsupported: src/aiconfigurator/systems/data/b60/oneccl/2021.17.2/oneccl_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b60/vllm/0.12.0/context_attention_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b60/vllm/0.12.0/custom_allreduce_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b60/vllm/0.12.0/gemm_perf.parquet
[WARN] Unsupported: src/aiconfi...
GitHub Actions: Copyright Checks / 0_copyright-checks.txt: feat(afd): add AFD default CLI mode built on the v2 Task architecture
Conclusion: failure
erf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/trtllm/1.3.0rc10/scale_matrix_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/trtllm/1.3.0rc10/wideep_moe_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/trtllm/1.3.0rc15/SHARED_LAYER_REUSE.txt
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/vllm/0.19.0/context_attention_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/vllm/0.19.0/custom_allreduce_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/vllm/0.19.0/dsa_context_module_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/vllm/0.19.0/dsa_generation_module_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/vllm/0.19.0/encoder_attention_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/vllm/0.19.0/gdn_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/vllm/0.19.0/gemm_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/vllm/0.19.0/generation_attention_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/vllm/0.19.0/mla_context_module_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/vllm/0.19.0/mla_generation_module_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/vllm/0.19.0/moe_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/vllm/0.22.0/SHARED_LAYER_REUSE.txt
[WARN] Unsupported: src/aiconfigurator/systems/data/b60/oneccl/2021.17.2/oneccl_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b60/vllm/0.12.0/context_attention_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b60/vllm/0.12.0/custom_allreduce_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b60/vllm/0.12.0/gemm_perf.parquet
[WARN] Unsupported: src/aiconfi...
🧰 Additional context used
📓 Path-based instructions (8)
src/aiconfigurator/generator/**/*.py
📄 CodeRabbit inference engine (.claude/rules/generator/debugging.md)
When debugging generator output issues, verify template variable names match the rendering context in
src/aiconfigurator/generator/rendering/engine.py:make_worker_context()
Files:
src/aiconfigurator/generator/facts/request_resolution.py
**/*.py
📄 CodeRabbit inference engine (.claude/rules/generator/guard_rails.md)
**/*.py: Catch unsupported system/backend/version combinations early with fast validation to prevent wasting user time on late failures after profiling.
MoE model detection must useconfig.json, not model name patterns, to correctly identify MoE models with non-standard naming.
Usemodel_familynotmodel_namefor model compatibility checks;model_nameincludes size/quantization info that changes across variants.
safe_model_namemust be generated BEFORE saving results to avoid race condition with rawmodel_pathcontaining slashes.
Quantization type must be inferred from model config, not name patterns, to handle inconsistent naming conventions.
Files:
src/aiconfigurator/generator/facts/request_resolution.pysrc/aiconfigurator/sdk/picking.pysrc/aiconfigurator/cli/utils.pysrc/aiconfigurator/sdk/inference_session.pysrc/aiconfigurator/sdk/sweep.pytests/unit/cli/test_afd_default_cli.pytests/unit/cli/test_cli_workflow.pytests/unit/sdk/test_afd_parallel_lists.pytests/unit/sdk/test_afd_pareto.pytests/unit/sdk/test_afd_pareto_review_actions.pysrc/aiconfigurator/cli/report_and_save.pysrc/aiconfigurator/sdk/common.pysrc/aiconfigurator/sdk/pareto_analysis.pysrc/aiconfigurator/cli/main.pysrc/aiconfigurator/sdk/task_v2.py
src/aiconfigurator/generator/**
📄 CodeRabbit inference engine (AGENTS.md)
Before making any change under
src/aiconfigurator/generator/**, you must read.claude/rules/generator-development.mdfirst.
Files:
src/aiconfigurator/generator/facts/request_resolution.pysrc/aiconfigurator/generator/facts/hardware.yaml
⚙️ CodeRabbit configuration file
src/aiconfigurator/generator/**: - Enforce the generator development rules from.claude/rules/generator-development.md.
- Trace changes across the pipeline: input parsing, defaults, rule evaluation, parameter mapping, template rendering, and emitted artifacts.
- For
.j2templates and.rulefiles, verify# Guard:and# Why:comments are preserved and followed. Flag removals or bypasses unless the PR explicitly justifies them.- Check cross-backend consistency across TRT-LLM, vLLM, and SGLang when a backend-specific rule, template, parameter, or versioned artifact changes.
- For new user-facing generator parameters, require matching CLI, SDK/profiler bridge, generator validator, docs, and test coverage updates where applicable.
- For versioned template changes, verify the affected backend versions against
src/aiconfigurator/generator/config/backend_version_matrix.yaml.
Files:
src/aiconfigurator/generator/facts/request_resolution.pysrc/aiconfigurator/generator/facts/hardware.yaml
**/*
⚙️ CodeRabbit configuration file
**/*: - Prefer applicable inline comments. When the correct fix is clear, small, and limited to the commented diff hunk, include it as a GitHub Suggested Change so the author can apply it with one click.
- Do not use a suggested change when the fix requires broader design choices, multiple files, generated artifacts, unavailable context, or validation that cannot be inferred from the diff.
- If a comment is not directly applicable, state the smallest concrete next step and why a one-click suggestion is not safe.
Files:
src/aiconfigurator/generator/facts/request_resolution.pysrc/aiconfigurator/sdk/picking.pysrc/aiconfigurator/cli/utils.pysrc/aiconfigurator/sdk/inference_session.pysrc/aiconfigurator/sdk/sweep.pytests/unit/cli/test_afd_default_cli.pytests/unit/cli/test_cli_workflow.pytests/unit/sdk/test_afd_parallel_lists.pytests/unit/sdk/test_afd_pareto.pytests/unit/sdk/test_afd_pareto_review_actions.pysrc/aiconfigurator/cli/report_and_save.pysrc/aiconfigurator/sdk/common.pysrc/aiconfigurator/sdk/pareto_analysis.pysrc/aiconfigurator/cli/main.pysrc/aiconfigurator/generator/facts/hardware.yamlsrc/aiconfigurator/sdk/task_v2.py
src/aiconfigurator/sdk/**
⚙️ CodeRabbit configuration file
src/aiconfigurator/sdk/**: - Verify SDK API changes remain compatible with generator inputs, profiler data flow, and documented examples.
- Flag silent schema or field-name drift between SDK models and generator/module bridge code.
Files:
src/aiconfigurator/sdk/picking.pysrc/aiconfigurator/sdk/inference_session.pysrc/aiconfigurator/sdk/sweep.pysrc/aiconfigurator/sdk/common.pysrc/aiconfigurator/sdk/pareto_analysis.pysrc/aiconfigurator/sdk/task_v2.py
src/aiconfigurator/cli/**
⚙️ CodeRabbit configuration file
src/aiconfigurator/cli/**: - Check that CLI argument changes preserve backward compatibility, validation behavior, defaults, and plain-output expectations.
- For new or changed user-facing options, verify docs updates and generator/SDK wiring.
- Watch for non-TTY assumptions in tests or output formatting.
Files:
src/aiconfigurator/cli/utils.pysrc/aiconfigurator/cli/report_and_save.pysrc/aiconfigurator/cli/main.py
tests/**
⚙️ CodeRabbit configuration file
tests/**: - Check that tests cover the changed behavior rather than only the happy path.
- Watch for fixtures or golden outputs that mask backend drift, support-matrix ordering changes, or CLI output regressions.
Files:
tests/unit/cli/test_afd_default_cli.pytests/unit/cli/test_cli_workflow.pytests/unit/sdk/test_afd_parallel_lists.pytests/unit/sdk/test_afd_pareto.pytests/unit/sdk/test_afd_pareto_review_actions.py
**/*.{yaml,yml,json}
📄 CodeRabbit inference engine (.claude/rules/generator/guard_rails.md)
**/*.{yaml,yml,json}:build_confignesting is version-dependent: Pre-1.2.0rc5 places it insidebuild_config. Post-1.2.0rc5 places it at top-level. Wrong placement is silently ignored.
Template variables must be top-level keys; nested paths likebuild_config.max_batch_sizeare undefined becauseengine.pyflattens the context.
cache_transceiver_configlifecycle: Absent pre-1.0.0rc4. In engine YAML 1.0.0rc4-1.3.0rc1. In CLI--override-engine-argspost-1.3.0rc5.
cache_transceiver_config.backendmust be set to'DEFAULT'or omission causes TRT-LLM to reject config in disagg mode.
Engine params must go into--override-engine-argsJSON; TRT-LLM's Dynamo argparser only accepts specific direct CLI flags.
max_num_tokens % tokens_per_block == 0must be true; TRT-LLM asserts block alignment and violation crashes engine startup.
cache_transceiver_max_tokens_in_buffermust align to block size; same block-alignment assertion applies asmax_num_tokens.
Prefill deployment: Setdisable_overlap_scheduler = trueto prevent hangs caused by overlap scheduler on prefill.
Decode/aggregation deployment: Setdisable_overlap_scheduler = falseto avoid performance degradation.
MoE models: EnsureTP = moe_tp * moe_epto prevent out-of-memory errors from model replication per GPU.
KV cache dtype: Convert literal"float16"or"bfloat16"to"auto"for TRT-LLM compatibility; literal strings are not accepted.
SGLang backend: Do NOT emit--moe-dense-tp-size; SGLang only accepts value 1 or None, other values crash startup.
SGLang backend: KV transfer backend default must be"nixl"for disaggregated mode (SGLang >=0.5.6.post2 requires explicit backend).
SGLang aggregation mode: Setenable_mixed_chunk = trueto avoid poor prefill/decode scheduling performance.
SGLang backend: Convert KV cache dtype"fp8"to"fp8_e4m3"; SGLang does not accept the literal"fp8"string.
SGLang backend: Convert KV cache dtype"bfloat16"to"auto"; SGLang interprets the li...
Files:
src/aiconfigurator/generator/facts/hardware.yaml
🧠 Learnings (4)
📚 Learning: 2026-03-25T12:18:21.280Z
Learnt from: Arsene12358
Repo: ai-dynamo/aiconfigurator PR: 651
File: src/aiconfigurator/sdk/models/base.py:197-205
Timestamp: 2026-03-25T12:18:21.280Z
Learning: In `src/aiconfigurator/sdk/models/base.py` inside `check_is_moe` for the NEMOTRONH model family, treat `extra_params` (accessed as `n` in-code) as always being a `common.NemotronHConfig` object, not a `dict`. This is constructed explicitly in `src/aiconfigurator/sdk/utils.py` for the `NemotronHForCausalLM` architecture. Therefore the existing `hasattr()`-based check is appropriate and you should not add or keep any dict-handling branch for `extra_params` in this code path.
Applied to files:
.gitignore
📚 Learning: 2026-05-13T13:41:38.843Z
Learnt from: tianhaox
Repo: ai-dynamo/aiconfigurator PR: 1002
File: collector/vllm/collect_dsv4_attn.py:778-781
Timestamp: 2026-05-13T13:41:38.843Z
Learning: In `collector/vllm/collect_dsv4_attn.py`, reviewers should treat the `query_len` argument of `_bench_paged_mqa_logits_kernel` as the kernel’s M dimension (the total packed query rows across the whole batch, i.e., `batch_size * isl`), not a per-request sequence length. In this codepath, `fp8_fp4_paged_mqa_logits` is dispatched once with `q` shaped `(num_total_query_tokens, 1, n_heads, head_dim)`, so passing `num_tokens = batch_size * isl` as the kernel’s `query_len` (M) is correct. Do not flag this as a bug or suggest passing `isl` alone; this aligns with the SGLang reference behavior in `collector/sglang/deepseekv4_sparse_modules.py` (where `batch_size` is explicitly accounted for and `full_s = M + past_kv`).
Applied to files:
.gitignore
📚 Learning: 2026-05-16T12:55:06.728Z
Learnt from: tianhaox
Repo: ai-dynamo/aiconfigurator PR: 1080
File: src/aiconfigurator/sdk/perf_database.py:4046-4053
Timestamp: 2026-05-16T12:55:06.728Z
Learning: In src/aiconfigurator/sdk/perf_database.py, preserve PerfDatabase._interp_pr’s behavior and signature: it intentionally tags source="silicon" for any results derived from silicon tables and silicon-based derivations, including MoE overflow/extrapolation. Do not modify _interp_pr to accept a new source parameter unless the maintainer explicitly requests it.
Applied to files:
.gitignore
📚 Learning: 2026-05-01T00:39:37.334Z
Learnt from: simone-chen
Repo: ai-dynamo/aiconfigurator PR: 956
File: src/aiconfigurator/sdk/perf_database.py:4144-4151
Timestamp: 2026-05-01T00:39:37.334Z
Learning: In src/aiconfigurator/sdk/**/*.py, preserve upstream metadata for PerformanceResult.source: since PerformanceResult defaults source to "silicon", callers should not force-set result.source (e.g., PerfDatabase._query_silicon_or_hybrid should rely on the default and keep any existing source information rather than overwriting it). Only set source explicitly when you truly intend to change it.
Applied to files:
src/aiconfigurator/sdk/picking.pysrc/aiconfigurator/sdk/inference_session.pysrc/aiconfigurator/sdk/sweep.pysrc/aiconfigurator/sdk/common.pysrc/aiconfigurator/sdk/pareto_analysis.pysrc/aiconfigurator/sdk/task_v2.py
🪛 ast-grep (0.44.1)
src/aiconfigurator/sdk/task_v2.py
[warning] 407-407: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(yaml_path)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
🪛 GitHub Actions: Copyright Checks / 0_copyright-checks.txt
src/aiconfigurator/generator/facts/hardware.yaml
[error] 1-1: Copyright checkers: [FAIL] invalid/missing header.
🪛 GitHub Actions: Copyright Checks / copyright-checks
src/aiconfigurator/generator/facts/hardware.yaml
[error] 1-1: Copyright check failed (basic). Invalid/missing SPDX copyright header.
🪛 GitHub Actions: Lint and Format / 0_Lint and Format (Ruff).txt
tests/unit/sdk/test_afd_pareto.py
[error] 264-264: ruff (RUF043): Pattern passed to match= contains metacharacters but is neither escaped nor raw. Use a raw string or re.escape()
src/aiconfigurator/sdk/pareto_analysis.py
[error] 702-702: ruff (N806): Variable _BS_LO in function should be lowercase
[error] 702-702: ruff (N806): Variable _BS_HI in function should be lowercase
🪛 GitHub Actions: Lint and Format / Lint and Format (Ruff)
tests/unit/sdk/test_afd_pareto.py
[error] 264-264: ruff check: RUF043 Pattern passed to match= contains metacharacters but is neither escaped nor raw. Use a raw string or re.escape().
src/aiconfigurator/sdk/pareto_analysis.py
[error] 702-702: ruff check: N806 Variable _BS_LO in function should be lowercase
[error] 702-702: ruff check: N806 Variable _BS_HI in function should be lowercase
🔇 Additional comments (18)
src/aiconfigurator/generator/facts/hardware.yaml (1)
1-90: LGTM!src/aiconfigurator/generator/facts/request_resolution.py (1)
37-37: LGTM!src/aiconfigurator/cli/main.py (1)
33-33: LGTM!Also applies to: 223-234, 1185-1199, 1210-1239, 1251-1258, 1278-1299, 1359-1426, 1439-1442, 1491-1491, 2461-2461
src/aiconfigurator/cli/report_and_save.py (2)
537-541: LGTM!
141-211: 🩺 Stability & AvailabilityDrop this check
ColumnsAFDalready includes(a)workersand(f)workers, and the AFD summary uses those same fields, so this won’t raise aKeyError.> Likely an incorrect or invalid review comment.src/aiconfigurator/cli/utils.py (1)
182-201: LGTM!.gitignore (1)
41-41: LGTM!tests/unit/cli/test_afd_default_cli.py (1)
1-229: LGTM!tests/unit/cli/test_cli_workflow.py (1)
126-150: LGTM!tests/unit/sdk/test_afd_parallel_lists.py (1)
1-98: LGTM!tests/unit/sdk/test_afd_pareto.py (1)
1-262: LGTM!Also applies to: 275-296
tests/unit/sdk/test_afd_pareto_review_actions.py (1)
1-336: LGTM!src/aiconfigurator/sdk/common.py (1)
870-891: LGTM!Also applies to: 1102-1102
src/aiconfigurator/sdk/inference_session.py (1)
1854-1859: LGTM!src/aiconfigurator/sdk/sweep.py (1)
964-1062: LGTM!src/aiconfigurator/sdk/task_v2.py (2)
306-393: LGTM!Also applies to: 442-442, 570-607, 683-695, 701-753, 794-794, 830-830, 843-928, 930-932, 1186-1218, 1343-1395, 1415-1417, 1692-1742, 1790-1807
395-417: 🔒 Security & PrivacyConfirm
system_nameis path-safe before joining it into the YAML path. If this value can come from a wrapping API/service, reject separators like/and..(or resolve and enforce containment) before reading from disk.src/aiconfigurator/sdk/picking.py (1)
208-212: LGTM!Also applies to: 321-324, 547-548
| exp_config.get("model_path") or exp_config.get("prefill_model_path") or exp_config.get("decode_model_path") | ||
| ) | ||
| if serving_mode not in {"agg", "disagg"} or not model_path: | ||
| if serving_mode not in {"agg", "disagg", "afd"} or not model_path: |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n --iglob '*.md' -C2 'serving.mode|serving_mode' docs/ README.md 2>/dev/null
fd -e yaml -e yml . | xargs rg -l 'serving_mode' 2>/dev/nullRepository: ai-dynamo/aiconfigurator
Length of output: 5414
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## afd occurrences\n'
rg -n --hidden --glob '!**/.git/**' '\bafd\b' README.md docs src/aiconfigurator/cli || true
printf '\n## example yaml\n'
cat -n src/aiconfigurator/cli/example.yaml
printf '\n## docs snippets\n'
sed -n '1,120p' docs/advanced_tuning.md
printf '\n--- cli_user_guide part 1 ---\n'
sed -n '880,1080p' docs/cli_user_guide.md
printf '\n--- cli_user_guide part 2 ---\n'
sed -n '1080,1180p' docs/cli_user_guide.mdRepository: ai-dynamo/aiconfigurator
Length of output: 33061
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1470,1585p' src/aiconfigurator/cli/main.pyRepository: ai-dynamo/aiconfigurator
Length of output: 6273
Add afd to the experiment YAML docs.
serving_mode: afd is accepted here, but the source docs/templates still only show agg and disagg. Add afd to src/aiconfigurator/cli/example.yaml and the experiment sections in docs/cli_user_guide.md / docs/advanced_tuning.md so users see it as a valid serving_mode.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/aiconfigurator/cli/main.py` at line 1505, The serving_mode validation in
main.py already accepts afd, but the user-facing experiment examples and docs
still only document agg and disagg. Update the serving_mode entries in
src/aiconfigurator/cli/example.yaml and the experiment sections in
docs/cli_user_guide.md and docs/advanced_tuning.md to include afd alongside the
existing modes so users see it as a valid option.
Source: Path instructions
| try: | ||
| task_config = {**exp_config, "database_mode": database_mode} | ||
| tasks[exp_name] = Task.from_yaml(task_config, **overrides) | ||
| if serving_mode == "afd": | ||
| if engine_step_backend is not None and "engine_step_backend" not in task_config: | ||
| task_config["engine_step_backend"] = engine_step_backend | ||
| # Build a v2 Task for AFD from YAML config | ||
| afd_kwargs = { | ||
| "serving_mode": "afd", | ||
| "model_path": task_config["model_path"], | ||
| "system_name": task_config["system_name"], | ||
| "backend_name": task_config.get("backend_name", common.BackendName.trtllm.value), | ||
| "backend_version": task_config.get("backend_version"), | ||
| "isl": task_config.get("isl", 4000), | ||
| "osl": task_config.get("osl", 1000), | ||
| "image_height": task_config.get("image_height", 0), | ||
| "image_width": task_config.get("image_width", 0), | ||
| "num_images_per_request": task_config.get( | ||
| "num_images_per_request", task_config.get("num_images", 1) | ||
| ), | ||
| "prefix": task_config.get("prefix", 0), | ||
| "ttft": task_config.get("ttft", 1000), | ||
| "tpot": task_config.get("tpot", 50), | ||
| "request_latency": task_config.get("request_latency"), | ||
| "enable_wideep": task_config.get("enable_wideep", False), | ||
| "enable_chunked_prefill": task_config.get("enable_chunked_prefill", False), | ||
| "moe_backend": task_config.get("moe_backend"), | ||
| "total_gpus": task_config.get("total_gpus"), | ||
| "database_mode": database_mode, | ||
| "free_gpu_memory_fraction": task_config.get("free_gpu_memory_fraction"), | ||
| "max_seq_len": task_config.get("max_seq_len"), | ||
| "engine_step_backend": task_config.get("engine_step_backend"), | ||
| } | ||
| tasks[exp_name] = Task(**{k: v for k, v in afd_kwargs.items() if v is not None}) | ||
| else: | ||
| tasks[exp_name] = Task.from_yaml(task_config, **overrides) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
AFD experiment construction silently drops any YAML fields not in the hardcoded whitelist, unlike the strict Task.from_yaml path used for agg/disagg.
Task.from_yaml (used for agg/disagg) explicitly rejects unknown/unconsumed keys ("there is no silent-ignore path" — see task_v2.py:from_yaml). The AFD branch instead builds a fixed afd_kwargs dict from a curated subset of fields (no nextn, nextn_accept_rates, or any *_quant_mode fields), then constructs Task(**afd_kwargs) directly. Any of those fields set by a user in an AFD experiment YAML entry are silently ignored with no warning or error — a real behavioral inconsistency between serving modes in the same experiment file, and a violation of the "no silent-ignore" contract that from_yaml was designed to guarantee.
Consider either extending afd_kwargs to include the missing fields, or routing AFD through Task.from_yaml-style key validation so unconsumed keys raise instead of being dropped.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/aiconfigurator/cli/main.py` around lines 1556 - 1590, The AFD branch in
main.py builds a fixed afd_kwargs whitelist and then calls Task(**...), which
silently drops YAML fields that Task.from_yaml would reject. Update the AFD
construction path to either include the missing AFD-related keys such as nextn,
nextn_accept_rates, and any *_quant_mode fields, or add Task.from_yaml-style
validation so unconsumed keys raise instead of being ignored. Keep the behavior
consistent with the existing agg/disagg path and use the Task/Task.from_yaml and
afd_kwargs logic in this function to locate the fix.
| gpus_per_node = _lookup_num_gpus_per_node(self.system_name) | ||
| if gpus_per_node is None: | ||
| raise ValueError( | ||
| f"Cannot resolve num_gpus_per_node for system '{self.system_name}'; " | ||
| "AFD requires a valid system yaml spec." | ||
| ) | ||
| self._afd_gpus_per_node = gpus_per_node | ||
|
|
||
| # Pinned topology: skip sweep enumeration | ||
| if self.afd_n_a_nodes is not None and self.afd_n_f_nodes is not None and self.afd_tp_a is not None: | ||
| self._afd_parallel_config_list = [] # empty = single-point mode | ||
| return |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Pinned AFD topology doesn't validate afd_tp_a divides gpus_per_node.
In the swept path, build_afd_parallel_lists only emits tp_a values where gpus_per_node % tp_a == 0. But the pinned single-point path (_run_afd_single_point, line 2027: n_a_workers = (n_a_nodes * gpus_per_node) // tp_a) has no such guarantee — an operator-supplied afd_tp_a that doesn't evenly divide gpus_per_node silently truncates n_a_workers, producing a wrong GPU count and throughput/latency estimate with no warning. This is only caught indirectly when afd_total_batch_size is also set (via the n_a_workers % ... != 0 check); the default-batch path (a_batch_size = self.afd_a_batch_size or 128) has no protection at all.
Add the check where the topology is confirmed pinned, right after gpus_per_node is resolved, so the failure surfaces immediately instead of after a (potentially expensive) profiling run.
🛡️ Proposed fast-fail validation
# Pinned topology: skip sweep enumeration
if self.afd_n_a_nodes is not None and self.afd_n_f_nodes is not None and self.afd_tp_a is not None:
+ if gpus_per_node % self.afd_tp_a != 0:
+ raise ValueError(
+ f"afd_tp_a={self.afd_tp_a} must evenly divide gpus_per_node={gpus_per_node} "
+ "for a pinned AFD topology."
+ )
self._afd_parallel_config_list = [] # empty = single-point mode
returnAs per coding guidelines, "Catch unsupported system/backend/version combinations early with fast validation to prevent wasting user time on late failures after profiling" — the same fail-fast principle applies to invalid pinned-topology parameters.
Also applies to: 2010-2049
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/aiconfigurator/sdk/task_v2.py` around lines 1108 - 1119, The pinned AFD
topology path in task_v2.py accepts afd_tp_a without validating that it evenly
divides gpus_per_node, which can lead to silent truncation in
_run_afd_single_point and incorrect worker/GPU counts. Add a fast-fail
validation immediately after _lookup_num_gpus_per_node(self.system_name)
resolves gpus_per_node and before the pinned-topology early return, checking
that self.afd_tp_a divides gpus_per_node whenever afd_n_a_nodes and
afd_n_f_nodes are also pinned. Raise a clear ValueError from this validation so
invalid pinned settings are rejected before any profiling or single-point
execution begins.
Source: Coding guidelines
| # Also resolve disagg-style prefill parallel lists for combined-with-PD | ||
| if self.afd_combined_with_pd: | ||
| prefill_cfg, _ = build_disagg_parallel_lists( | ||
| backend_name=self.backend_name, | ||
| is_moe=self._is_moe, | ||
| prefill_system=self.system_name, | ||
| decode_system=self.system_name, | ||
| prefill_enable_wideep=self.enable_wideep, | ||
| decode_enable_wideep=self.enable_wideep, | ||
| moe_backend=self.moe_backend, | ||
| ) | ||
| # Store prefill parallel for sweep_afd_kwargs | ||
| self.prefill_num_gpu_candidates = prefill_cfg["num_gpu_per_worker"] | ||
| self.prefill_tp_candidates = prefill_cfg["tp_list"] | ||
| self.prefill_pp_candidates = prefill_cfg["pp_list"] | ||
| self.prefill_dp_candidates = prefill_cfg["dp_list"] | ||
| self.prefill_moe_tp_candidates = prefill_cfg["moe_tp_list"] | ||
| self.prefill_moe_ep_candidates = prefill_cfg["moe_ep_list"] |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
User-supplied AFD prefill search-space fields are silently overwritten.
prefill_num_gpu_candidates/prefill_tp_candidates/prefill_pp_candidates/prefill_dp_candidates/prefill_moe_tp_candidates/prefill_moe_ep_candidates are unconditionally reassigned here, discarding any value the user set explicitly in YAML/CLI for the AFD combined-with-PD static prefill pool. The quant-mode propagation loop immediately below (lines 1178-1184) correctly guards with if getattr(self, f"prefill_{qkey}") is None, and _fill_role_search (used by disagg) follows the same "user override wins" discipline — this block should match.
🐛 Proposed fix to respect explicit user overrides
# Store prefill parallel for sweep_afd_kwargs
- self.prefill_num_gpu_candidates = prefill_cfg["num_gpu_per_worker"]
- self.prefill_tp_candidates = prefill_cfg["tp_list"]
- self.prefill_pp_candidates = prefill_cfg["pp_list"]
- self.prefill_dp_candidates = prefill_cfg["dp_list"]
- self.prefill_moe_tp_candidates = prefill_cfg["moe_tp_list"]
- self.prefill_moe_ep_candidates = prefill_cfg["moe_ep_list"]
+ if self.prefill_num_gpu_candidates is None:
+ self.prefill_num_gpu_candidates = prefill_cfg["num_gpu_per_worker"]
+ if self.prefill_tp_candidates is None:
+ self.prefill_tp_candidates = prefill_cfg["tp_list"]
+ if self.prefill_pp_candidates is None:
+ self.prefill_pp_candidates = prefill_cfg["pp_list"]
+ if self.prefill_dp_candidates is None:
+ self.prefill_dp_candidates = prefill_cfg["dp_list"]
+ if self.prefill_moe_tp_candidates is None:
+ self.prefill_moe_tp_candidates = prefill_cfg["moe_tp_list"]
+ if self.prefill_moe_ep_candidates is None:
+ self.prefill_moe_ep_candidates = prefill_cfg["moe_ep_list"]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Also resolve disagg-style prefill parallel lists for combined-with-PD | |
| if self.afd_combined_with_pd: | |
| prefill_cfg, _ = build_disagg_parallel_lists( | |
| backend_name=self.backend_name, | |
| is_moe=self._is_moe, | |
| prefill_system=self.system_name, | |
| decode_system=self.system_name, | |
| prefill_enable_wideep=self.enable_wideep, | |
| decode_enable_wideep=self.enable_wideep, | |
| moe_backend=self.moe_backend, | |
| ) | |
| # Store prefill parallel for sweep_afd_kwargs | |
| self.prefill_num_gpu_candidates = prefill_cfg["num_gpu_per_worker"] | |
| self.prefill_tp_candidates = prefill_cfg["tp_list"] | |
| self.prefill_pp_candidates = prefill_cfg["pp_list"] | |
| self.prefill_dp_candidates = prefill_cfg["dp_list"] | |
| self.prefill_moe_tp_candidates = prefill_cfg["moe_tp_list"] | |
| self.prefill_moe_ep_candidates = prefill_cfg["moe_ep_list"] | |
| # Also resolve disagg-style prefill parallel lists for combined-with-PD | |
| if self.afd_combined_with_pd: | |
| prefill_cfg, _ = build_disagg_parallel_lists( | |
| backend_name=self.backend_name, | |
| is_moe=self._is_moe, | |
| prefill_system=self.system_name, | |
| decode_system=self.system_name, | |
| prefill_enable_wideep=self.enable_wideep, | |
| decode_enable_wideep=self.enable_wideep, | |
| moe_backend=self.moe_backend, | |
| ) | |
| # Store prefill parallel for sweep_afd_kwargs | |
| if self.prefill_num_gpu_candidates is None: | |
| self.prefill_num_gpu_candidates = prefill_cfg["num_gpu_per_worker"] | |
| if self.prefill_tp_candidates is None: | |
| self.prefill_tp_candidates = prefill_cfg["tp_list"] | |
| if self.prefill_pp_candidates is None: | |
| self.prefill_pp_candidates = prefill_cfg["pp_list"] | |
| if self.prefill_dp_candidates is None: | |
| self.prefill_dp_candidates = prefill_cfg["dp_list"] | |
| if self.prefill_moe_tp_candidates is None: | |
| self.prefill_moe_tp_candidates = prefill_cfg["moe_tp_list"] | |
| if self.prefill_moe_ep_candidates is None: | |
| self.prefill_moe_ep_candidates = prefill_cfg["moe_ep_list"] |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/aiconfigurator/sdk/task_v2.py` around lines 1160 - 1177, The AFD
combined-with-PD prefill search-space values are being overwritten
unconditionally in the prefill resolution block, so user-specified YAML/CLI
overrides are lost. Update the logic in the combined-with-PD handling around the
build_disagg_parallel_lists call so each prefill_*_candidates field is only
assigned when the existing attribute is None, matching the guard used in the
quant-mode propagation loop and the override-preserving behavior in
_fill_role_search.
| def test_prefill_candidate_overflow_errors(self): | ||
| with pytest.raises(ValueError, match="afd_config.prefill_search.max_candidates=1"): | ||
| _enumerate_afd_prefill_options( | ||
| model_path="Qwen/Qwen3-32B", | ||
| runtime_config=type("RuntimeConfig", (), {"batch_size": 1})(), | ||
| database=object(), | ||
| backend_name="trtllm", | ||
| gpus_per_node=8, | ||
| prefill_parallel_config_list=[(1, 1, 1, 1, 1)], | ||
| prefill_batch_size_list=[1, 2], | ||
| max_candidates=1, | ||
| ) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Fix RUF043: unescaped regex metacharacters in match=.
Pipeline lint (Ruff RUF043) fails here: . in "afd_config.prefill_search.max_candidates=1" is an unescaped regex metacharacter passed to pytest.raises(match=...).
🔧 Proposed fix
+import re
+
def test_prefill_candidate_overflow_errors(self):
- with pytest.raises(ValueError, match="afd_config.prefill_search.max_candidates=1"):
+ with pytest.raises(ValueError, match=re.escape("afd_config.prefill_search.max_candidates=1")):📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def test_prefill_candidate_overflow_errors(self): | |
| with pytest.raises(ValueError, match="afd_config.prefill_search.max_candidates=1"): | |
| _enumerate_afd_prefill_options( | |
| model_path="Qwen/Qwen3-32B", | |
| runtime_config=type("RuntimeConfig", (), {"batch_size": 1})(), | |
| database=object(), | |
| backend_name="trtllm", | |
| gpus_per_node=8, | |
| prefill_parallel_config_list=[(1, 1, 1, 1, 1)], | |
| prefill_batch_size_list=[1, 2], | |
| max_candidates=1, | |
| ) | |
| import re | |
| def test_prefill_candidate_overflow_errors(self): | |
| with pytest.raises(ValueError, match=re.escape("afd_config.prefill_search.max_candidates=1")): | |
| _enumerate_afd_prefill_options( | |
| model_path="Qwen/Qwen3-32B", | |
| runtime_config=type("RuntimeConfig", (), {"batch_size": 1})(), | |
| database=object(), | |
| backend_name="trtllm", | |
| gpus_per_node=8, | |
| prefill_parallel_config_list=[(1, 1, 1, 1, 1)], | |
| prefill_batch_size_list=[1, 2], | |
| max_candidates=1, | |
| ) |
🧰 Tools
🪛 GitHub Actions: Lint and Format / 0_Lint and Format (Ruff).txt
[error] 264-264: ruff (RUF043): Pattern passed to match= contains metacharacters but is neither escaped nor raw. Use a raw string or re.escape()
🪛 GitHub Actions: Lint and Format / Lint and Format (Ruff)
[error] 264-264: ruff check: RUF043 Pattern passed to match= contains metacharacters but is neither escaped nor raw. Use a raw string or re.escape().
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/unit/sdk/test_afd_pareto.py` around lines 263 - 274, The pytest.raises
match string in test_prefill_candidate_overflow_errors uses unescaped regex
metacharacters, so Ruff flags it. Update the match argument in this test to
treat the literal config path from _enumerate_afd_prefill_options as plain text
by escaping the dots in afd_config.prefill_search.max_candidates, or otherwise
make the pattern a valid regex that matches the intended error message exactly.
Source: Pipeline failures
AIC Accuracy Regression TestingWorkflow result: Old revision: Summary
Comparison summary table
|
| "When 'auto' is used, selected serving-mode results are merged across backends and the " | ||
| "globally optimal configuration is selected. Default: trtllm.", | ||
| ) | ||
| parser.add_argument( |
There was a problem hiding this comment.
--serving-mode appears to be introduced by this PR. Previously, default always compared agg and disagg, while AFD was available only through single-point estimate mode.
Instead of adding a new auto/all selector, could we:
Keep serving_mode: afd available in exp for explicit or customized runs.
Have default automatically include AFD when it is supported and the GPU budget is at least two nodes; otherwise, continue comparing only agg and disagg.
This seems more consistent with the purpose of default: automatically compare all feasible serving strategies. Unsupported AFD configurations should be skipped with a clear warning rather than failing the entire default run.
We should also update the documentation/API accordingly and correct the PR description’s statement that the CLI surface is unchanged, since --serving-mode is new in this PR.
| default="auto", | ||
| help="Serving modes to sweep and compare. 'auto' (default) compares agg and disagg; " | ||
| "'all' compares agg, disagg, and afd; pick a single mode to restrict the search. " | ||
| "afd requires at least 2 nodes worth of GPUs and is skipped otherwise.", |
There was a problem hiding this comment.
Is the two-node minimum an intentional deployment limitation?
Bring AFD (Attention-FFN Disaggregated) to feature parity with the
default mode for Dense/PD by driving it through the v2 `Task` +
`sweep_afd` pipeline, removing the legacy `TaskConfig` / `TaskRunner`
adapter.
Users can now run:
aiconfigurator cli default --serving-mode afd ...
and get a Pareto sweep, SLA-aware picking, and a full report end-to-end
through the v2 Task stack.
Changes:
- task_v2.py: extend `Task` with `serving_mode='afd'` and 21 AFD fields;
add `_resolve_afd_search`, `sweep_afd_kwargs`, `_validate_afd`; migrate
`build_afd_parallel_lists` and normalize helpers off the legacy module.
- sweep.py / pareto_analysis.py: add `sweep_afd()` wrapping a new
`afd_pareto()` with KV-cache-derived batch sizing, combined-with-PD
static prefill, and SLA constraint enforcement.
- cli/main.py, report_and_save.py: remove `_LegacyAFDTaskAdapter`; drive
AFD through v2 `Task` in both default and experiment modes; add AFD
report rendering and summary comparison.
- tests: cover v2 AFD Task, parallel-list builder, pareto core, SLA /
review actions, and CLI integration.
CLI surface is unchanged; no changes to perf data or system YAML schema.
Signed-off-by: gehujun.ghj <gehujun.ghj@alibaba-inc.com>
Signed-off-by: lj382851 <lj382851@alibaba-inc.com>
Signed-off-by: yuxing.tyx <yuxing.tyx@alibaba-inc.com>
…ency The AFD decode path conflated a single micro-batch's full-layer latency (t_micro_step) with the global decode-step latency (TPOT). This broke the identity tokens/s == concurrency * tokens/s/user and made the prefill branch's t_step (num_layers * t_cycle, no M factor) inconsistent with decode. Changes: - inference_session.py: add _pipeline_global_step_latency returning (t_global_step, t_cycle, comm_hidden) where t_global_step = pipeline_fill + t_cycle * (M*L - 1); _integrate_decode_phase now returns the global step directly (plus t_cycle_avg and comm_hidden); _simulate_phase reuses the captured comm_hidden instead of re-calling _pipeline_tcycle; _build_summary uses tpot = decode t_step (now global), tokens/s = b_total / tpot, seq/s = tokens/s / (osl-1) (Little's law, matching agg). Report t_micro_step separately in result_dict and _PHASE_SCALAR_KEYS. - pareto_analysis.py: scale F-worker per-GPU batch cap by num_microbatches before comparing with A-side derived_bs, so the combined max batch口径 is consistent (A-side total vs F-side total). - task_v2.py: skip optimistic + num_microbatches < 3 degenerate candidates in build_afd_parallel_lists (they always fall back to conservative, duplicating candidates and flooding logs). - tests: update test_afd_parallel_lists (mb 2->3 for optimistic probe); add test_combine_scales_f_cap_by_microbatches in test_afd_pareto; add test_afd_summary_uses_global_batch_tpot_for_microbatch_pipeline and paired-scalars NaN coverage in test_afd_phase_completion; fixture supports t_micro_step. Signed-off-by: gehujun.ghj <gehujun.ghj@alibaba-inc.com>
644c8dd to
3707f59
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/unit/cli/test_afd_phase_completion.py (1)
414-450: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTest name implies integration but the two halves aren't actually linked in code.
This test calls
_pipeline_global_step_latencydirectly to prove the equation yields 26.0, then separately assertsrun_afd(phase="decode")propagatesdecode_metrics["t_step"]toresult["tpot"]. But_simulate_phaseis mocked here, so the actual code path that would call_pipeline_global_step_latencyto producet_stepinside_simulate_phaseis never exercised — the two assertions are only tied together by manually keepingt_step=26.0in sync with the equation's output. If_simulate_phase's real implementation diverges from_pipeline_global_step_latency(e.g. wrong args, wrong num_layers), this test wouldn't catch it.Consider either renaming the test to reflect that it validates two independent contracts, or adding a genuine integration test that exercises the real (unmocked)
_simulate_phase→_pipeline_global_step_latency→ summary chain.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/cli/test_afd_phase_completion.py` around lines 414 - 450, The test name suggests integration, but it separately validates the latency helper and mocked summary propagation without linking those code paths. Rename test_afd_summary_uses_global_batch_tpot_for_pipeline to describe the two independent contracts, or replace the mocked phase simulation with a genuine integration setup that exercises _simulate_phase, _pipeline_global_step_latency, and run_afd together.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/aiconfigurator/sdk/task_v2.py`:
- Around line 1187-1193: Update _resolve_afd_search to also propagate the
resolved AFD backend and WideEP settings to the prefill role, alongside the
quant modes. Set the corresponding prefill backend name and prefill WideEP flag
only when they are unset, so sweep_afd_kwargs, iter_parallel("prefill"), and
build_model_config(role="prefill") use the same AFD configuration without
overriding explicit prefill values.
---
Nitpick comments:
In `@tests/unit/cli/test_afd_phase_completion.py`:
- Around line 414-450: The test name suggests integration, but it separately
validates the latency helper and mocked summary propagation without linking
those code paths. Rename test_afd_summary_uses_global_batch_tpot_for_pipeline to
describe the two independent contracts, or replace the mocked phase simulation
with a genuine integration setup that exercises _simulate_phase,
_pipeline_global_step_latency, and run_afd together.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 78ce6d4c-e298-427a-97c5-22f02e5f277c
📒 Files selected for processing (17)
src/aiconfigurator/cli/main.pysrc/aiconfigurator/cli/report_and_save.pysrc/aiconfigurator/cli/utils.pysrc/aiconfigurator/generator/facts/hardware.yamlsrc/aiconfigurator/generator/facts/request_resolution.pysrc/aiconfigurator/sdk/common.pysrc/aiconfigurator/sdk/inference_session.pysrc/aiconfigurator/sdk/pareto_analysis.pysrc/aiconfigurator/sdk/picking.pysrc/aiconfigurator/sdk/sweep.pysrc/aiconfigurator/sdk/task_v2.pytests/unit/cli/test_afd_default_cli.pytests/unit/cli/test_afd_phase_completion.pytests/unit/cli/test_cli_workflow.pytests/unit/sdk/test_afd_parallel_lists.pytests/unit/sdk/test_afd_pareto.pytests/unit/sdk/test_afd_pareto_review_actions.py
✅ Files skipped from review due to trivial changes (1)
- src/aiconfigurator/sdk/picking.py
🚧 Files skipped from review as they are similar to previous changes (13)
- src/aiconfigurator/sdk/sweep.py
- tests/unit/sdk/test_afd_parallel_lists.py
- src/aiconfigurator/cli/utils.py
- src/aiconfigurator/generator/facts/request_resolution.py
- src/aiconfigurator/cli/report_and_save.py
- tests/unit/cli/test_cli_workflow.py
- src/aiconfigurator/generator/facts/hardware.yaml
- tests/unit/sdk/test_afd_pareto_review_actions.py
- tests/unit/sdk/test_afd_pareto.py
- src/aiconfigurator/sdk/common.py
- src/aiconfigurator/sdk/pareto_analysis.py
- tests/unit/cli/test_afd_default_cli.py
- src/aiconfigurator/cli/main.py
📜 Review details
⏰ Context from checks skipped due to timeout. (6)
- GitHub Check: Cargo Deny
- GitHub Check: Build and Test (unit)
- GitHub Check: Build and Test (e2e)
- GitHub Check: Rust/Python engine-step parity
- GitHub Check: AIC Accuracy Regression Testing
- GitHub Check: CodeRabbit / Review
⚠️ CI failures not shown inline (2)
GitHub Actions: Copyright Checks / copyright-checks: feat(afd): add AFD default CLI mode built on the v2 Task architecture
Conclusion: failure
der_attention_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/vllm/0.19.0/gdn_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/vllm/0.19.0/gemm_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/vllm/0.19.0/generation_attention_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/vllm/0.19.0/mla_context_module_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/vllm/0.19.0/mla_generation_module_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/vllm/0.19.0/moe_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/vllm/0.22.0/SHARED_LAYER_REUSE.txt
[WARN] Unsupported: src/aiconfigurator/systems/data/b60/oneccl/2021.17.2/oneccl_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b60/vllm/0.12.0/context_attention_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b60/vllm/0.12.0/custom_allreduce_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b60/vllm/0.12.0/gemm_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b60/vllm/0.12.0/generation_attention_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b60/vllm/0.12.0/moe_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b60/vllm/0.20.0/context_attention_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b60/vllm/0.20.0/custom_allreduce_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b60/vllm/0.20.0/gemm_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b60/vllm/0.20.0/generation_attention_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b60/vllm/0.20.0/moe_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/gb200/nccl/2.23/nccl_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/gb200/sglang/0.5.10/context_attention_perf.parquet
...
GitHub Actions: Copyright Checks / 0_copyright-checks.txt: feat(afd): add AFD default CLI mode built on the v2 Task architecture
Conclusion: failure
der_attention_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/vllm/0.19.0/gdn_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/vllm/0.19.0/gemm_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/vllm/0.19.0/generation_attention_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/vllm/0.19.0/mla_context_module_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/vllm/0.19.0/mla_generation_module_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/vllm/0.19.0/moe_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b300_sxm/vllm/0.22.0/SHARED_LAYER_REUSE.txt
[WARN] Unsupported: src/aiconfigurator/systems/data/b60/oneccl/2021.17.2/oneccl_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b60/vllm/0.12.0/context_attention_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b60/vllm/0.12.0/custom_allreduce_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b60/vllm/0.12.0/gemm_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b60/vllm/0.12.0/generation_attention_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b60/vllm/0.12.0/moe_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b60/vllm/0.20.0/context_attention_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b60/vllm/0.20.0/custom_allreduce_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b60/vllm/0.20.0/gemm_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b60/vllm/0.20.0/generation_attention_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/b60/vllm/0.20.0/moe_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/gb200/nccl/2.23/nccl_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/gb200/sglang/0.5.10/context_attention_perf.parquet
...
🧰 Additional context used
📓 Path-based instructions (4)
**/*.py
📄 CodeRabbit inference engine (.claude/rules/generator/guard_rails.md)
**/*.py: Catch unsupported system/backend/version combinations early with fast validation to prevent wasting user time on late failures after profiling.
MoE model detection must useconfig.json, not model name patterns, to correctly identify MoE models with non-standard naming.
Usemodel_familynotmodel_namefor model compatibility checks;model_nameincludes size/quantization info that changes across variants.
safe_model_namemust be generated BEFORE saving results to avoid race condition with rawmodel_pathcontaining slashes.
Quantization type must be inferred from model config, not name patterns, to handle inconsistent naming conventions.
Files:
tests/unit/cli/test_afd_phase_completion.pysrc/aiconfigurator/sdk/inference_session.pysrc/aiconfigurator/sdk/task_v2.py
**/*
⚙️ CodeRabbit configuration file
**/*: - Prefer applicable inline comments. When the correct fix is clear, small, and limited to the commented diff hunk, include it as a GitHub Suggested Change so the author can apply it with one click.
- Do not use a suggested change when the fix requires broader design choices, multiple files, generated artifacts, unavailable context, or validation that cannot be inferred from the diff.
- If a comment is not directly applicable, state the smallest concrete next step and why a one-click suggestion is not safe.
Files:
tests/unit/cli/test_afd_phase_completion.pysrc/aiconfigurator/sdk/inference_session.pysrc/aiconfigurator/sdk/task_v2.py
tests/**
⚙️ CodeRabbit configuration file
tests/**: - Check that tests cover the changed behavior rather than only the happy path.
- Watch for fixtures or golden outputs that mask backend drift, support-matrix ordering changes, or CLI output regressions.
Files:
tests/unit/cli/test_afd_phase_completion.py
src/aiconfigurator/sdk/**
⚙️ CodeRabbit configuration file
src/aiconfigurator/sdk/**: - Verify SDK API changes remain compatible with generator inputs, profiler data flow, and documented examples.
- Flag silent schema or field-name drift between SDK models and generator/module bridge code.
Files:
src/aiconfigurator/sdk/inference_session.pysrc/aiconfigurator/sdk/task_v2.py
🧠 Learnings (1)
📚 Learning: 2026-05-01T00:39:37.334Z
Learnt from: simone-chen
Repo: ai-dynamo/aiconfigurator PR: 956
File: src/aiconfigurator/sdk/perf_database.py:4144-4151
Timestamp: 2026-05-01T00:39:37.334Z
Learning: In src/aiconfigurator/sdk/**/*.py, preserve upstream metadata for PerformanceResult.source: since PerformanceResult defaults source to "silicon", callers should not force-set result.source (e.g., PerfDatabase._query_silicon_or_hybrid should rely on the default and keep any existing source information rather than overwriting it). Only set source explicitly when you truly intend to change it.
Applied to files:
src/aiconfigurator/sdk/inference_session.pysrc/aiconfigurator/sdk/task_v2.py
🪛 ast-grep (0.44.1)
src/aiconfigurator/sdk/task_v2.py
[warning] 416-416: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(yaml_path)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
🔇 Additional comments (4)
tests/unit/cli/test_afd_phase_completion.py (1)
503-511: LGTM!src/aiconfigurator/sdk/task_v2.py (2)
1126-1128: Pinned AFD topology still doesn't validateafd_tp_adividesgpus_per_node.The pinned early-return path skips the sweep's
gpus_per_node % tp_a == 0guarantee, so an operator-suppliedafd_tp_athat doesn't evenly dividegpus_per_nodesilently truncatesn_a_workersin_run_afd_single_point(Line 2036). Add a fast-fail check right here.
1180-1186: User-supplied AFD prefill search-space fields are still overwritten unconditionally.These
prefill_*_candidatesassignments discard explicit user overrides; guard each withis Noneto match the quant-mode loop below and_fill_role_search.src/aiconfigurator/sdk/inference_session.py (1)
1136-1159: LGTM!Also applies to: 1271-1372, 1490-1517, 1797-1912
| # Propagate resolved agg quant modes to prefill role so | ||
| # build_model_config(role="prefill") inherits the same promotions | ||
| # (e.g. GPT-OSS Blackwell w4a8_mxfp4_mxfp8) | ||
| for qkey in ("gemm_quant_mode", "moe_quant_mode", "kvcache_quant_mode", | ||
| "fmha_quant_mode", "comm_quant_mode"): | ||
| if getattr(self, f"prefill_{qkey}") is None: | ||
| setattr(self, f"prefill_{qkey}", getattr(self, qkey)) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm enumerate_parallel_config keys off backend + enable_wideep
rg -nP --type=py -C3 '\bdef enumerate_parallel_config\s*\(' src/aiconfigurator/sdk/utils.py
# Confirm build_model_config reads the prefill role's enable_wideep/backend
rg -nP --type=py -C2 "_role_attr\(role, \"enable_wideep\"\)|_role_attr\(role, \"backend_name\"\)" src/aiconfigurator/sdk/task_v2.pyRepository: ai-dynamo/aiconfigurator
Length of output: 3151
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the AFD/disagg parallel-list logic and prefill role handling.
sed -n '1120,1335p' src/aiconfigurator/sdk/task_v2.py | cat -n
# Inspect the rest of the AFD sweep path if needed.
sed -n '1335,1505p' src/aiconfigurator/sdk/task_v2.py | cat -nRepository: ai-dynamo/aiconfigurator
Length of output: 22772
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1120,1335p' src/aiconfigurator/sdk/task_v2.py | cat -n
sed -n '1335,1505p' src/aiconfigurator/sdk/task_v2.py | cat -nRepository: ai-dynamo/aiconfigurator
Length of output: 22772
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C4 'def sweep_afd_kwargs|prefill_backend_name|prefill_enable_wideep|afd_combined_with_pd|sweep_disagg_kwargs' src/aiconfigurator/sdk/task_v2.pyRepository: ai-dynamo/aiconfigurator
Length of output: 13838
Propagate AFD backend/wideep to the prefill role
_resolve_afd_search only copies the quant modes into prefill_*; sweep_afd_kwargs still calls iter_parallel("prefill") and build_model_config(role="prefill"), so combined-with-PD uses the default prefill_backend_name="trtllm" and prefill_enable_wideep=False even when AFD is running with backend_name="sglang" or WideEP enabled. That can drop valid prefill candidates and skew the sweep estimates.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/aiconfigurator/sdk/task_v2.py` around lines 1187 - 1193, Update
_resolve_afd_search to also propagate the resolved AFD backend and WideEP
settings to the prefill role, alongside the quant modes. Set the corresponding
prefill backend name and prefill WideEP flag only when they are unset, so
sweep_afd_kwargs, iter_parallel("prefill"), and
build_model_config(role="prefill") use the same AFD configuration without
overriding explicit prefill values.
tianhaox
left a comment
tianhaox
left a comment
There was a problem hiding this comment.
Reviewed this PR in depth (all findings below were independently re-verified against the PR branch before posting). The overall direction is right — making AFD a first-class v2 Task serving mode and deleting the legacy adapter is the correct architecture. However, verification found 8 confirmed correctness defects, two of which break features this PR itself advertises (pinned single-point estimation, large-cluster sweeps), so I'm requesting changes.
Confirmed defects (ranked by severity)
1. sdk/task_v2.py (~L2066) — pinned single-point AFD crashes 100% of the time.
_run_afd_single_point calls AFDConfig(..., n_a_workers=n_a_workers, ...), but AFDConfig.n_a_workers is declared field(init=False) (derived in __post_init__, config.py:260). Reproduced on this branch: TypeError: AFDConfig.__init__() got an unexpected keyword argument 'n_a_workers'. Any run with afd_n_a_nodes/afd_n_f_nodes/afd_tp_a set crashes — this path has no test coverage.
2. cli/main.py (~L1562) — the AFD experiment-YAML branch silently drops every afd_* field and nextn.
Instead of Task.from_yaml (used by the agg/disagg branch one line below), the AFD branch builds Task(**afd_kwargs) from a hard-coded 22-key whitelist. Pinned topology, afd_combined_with_pd, afd_total_batch_size, calibration factors, and nextn/nextn_accept_rates are all dropped with no warning — and this bypasses from_yaml's unknown-key ValueError guard, so users get a default full sweep that silently ignores their YAML. Since v2 Task already supports serving_mode='afd', the whole branch can collapse to Task.from_yaml(task_config, **overrides).
3. generator/module_bridge.py (~L191) — AFD results produce fabricated deployment artifacts.
task_config_to_generator_config only branches agg vs everything-else(=disagg). AFD rows have no (p)*/(d)* columns (ColumnsAFD uses (a)*/(f)*), and _series_val silently falls back to defaults, yielding a fictitious tp=1/pp=1/bs=1 disagg topology. save_results generates artifacts for every experiment unconditionally (report_and_save.py:845 → :1015), so with --save-dir set a bogus generator_config.yaml is written whenever AFD is feasible — AFD doesn't even need to win the comparison. At minimum, skip artifact generation for AFD with a warning.
4. sdk/task_v2.py (~L399) — default afd_max_candidates=2000 + candidate_overflow="error" makes AFD fail on ≥128-GPU clusters.
Counted with the actual enumeration logic: 128 GPUs dense = 2040 candidates > 2000 (MoE overflows at 96 GPUs with 3000+). The CLI exposes no override for afd_max_candidates/afd_candidate_overflow. Result: --serving-mode afd dies with SystemExit(1) ("No task configs could be built"); --serving-mode all silently drops AFD from the comparison. Suggest defaulting to truncate (or exposing the knobs on the CLI).
5. sdk/pareto_analysis.py (~L1284) — the quick balance-ratio prefilter probes the F pool at the wrong batch size, pruning genuinely balanced topologies.
The prefilter queries both pools at the same per-A-worker probe_bs, but the real evaluation feeds the F pool the aggregated tokens b_micro_total = n_a_workers * a_micro_batch_size (inference_session.py:991). t_f is underestimated by up to n_a_workers×, so a truly balanced candidate gets quick_ratio < 0.3 and is rejected (rejection_counts["balance"]) before run_afd ever runs — the optimal AFD topology can be silently excluded from the sweep. The probe's F-side batch should be scaled by the A-worker count.
6. sdk/task_v2.py (~L1714) — combined-with-PD prefill enumeration applies the wrong backend's parallel filters.
iter_parallel("prefill") reads prefill_backend_name (dataclass default "trtllm"), and _resolve_afd_search never copies the task's backend_name into it. For vLLM/SGLang AFD MoE tasks this applies trtllm's dp>1 and tp>1 pruning (dropping valid configs) while skipping vLLM's moe_tp>1 and moe_ep>1 unsupported filter (admitting configs the backend can't run). Fix: set prefill_backend_name = backend_name (and prefill_enable_wideep) during AFD resolution.
7. cli/main.py (~L1279) — regression (non-AFD): HYBRID mode lost the decode-system availability skip in --backend auto.
The pre-PR code used _database_mode_requires_declared_perf_database (covers SILICON and HYBRID) and warn-skipped backends whose decode system lacked data. The new _disagg_backend_available early-returns True for anything != SILICON. Under --backend auto --database-mode HYBRID with a decode system missing that backend's data, a disagg Task is now built with decode_backend_version=None and fails deep inside sweep_disagg(decode_database=None) with a raw AttributeError instead of the old clean skip. The condition should be restored to _database_mode_requires_declared_perf_database(database_mode).
8. sdk/task_v2.py (~L1813) — "empty candidate list" is overloaded as the pinned-topology sentinel.
build_afd_parallel_lists silently filters invalid user candidates (e.g. afd_tp_a_candidates=[3] on an 8-GPU/node system → [], log line only). An unpinned task with a legitimately empty search space is then misrouted into _run_afd_single_point (if not self._afd_parallel_config_list), which crashes with TypeError on n_f_nodes * gpus_per_node (None × int) instead of raising a clear no-feasible-config error. Use an explicit pinned flag and raise NoFeasibleConfigError for an empty candidate list.
Additional issues worth addressing
generator/facts/hardware.yamlwas rewritten via a YAML dump: 87 of the 117 removed lines are comments — the SPDX header and all provenance/guard comments (WIDEEP-crashes-on-Hopper rationale, NCCL env sources, node_selector provenance) are stripped. The h20/h20_3e addition is also unrelated to AFD; please hand-append just the new block (preserving comments) and consider splitting the hardware support into its own PR.- Docs: the new
--serving-modeflag is not documented indocs/cli_user_guide.md. - Third copy of the rate-matching calibration:
_AFD_PREFILL_DEGRADATION/_AFD_TTFT_CORRECTION_FACTORand the combine formula duplicatepicking.py/sweep.py, which are locked together by a parity test — the AFD copies sit outside that lock and will silently drift when the calibration is re-tuned. - Performance:
afd_paretonests the full candidate sweep inside the SLA-pair loop, rebuilding SLA-invariant per-candidate state (get_model×2, partitions, memory probes) per runtime config; each batch-size probe also constructs a freshAFDInferenceSession(re-running_build_models). Caching per candidate /topo_keywould cut wall-clock substantially on large sweeps.
Findings 1, 2, and 4 indicate the pinned-topology path, YAML experiment path, and large-cluster path have no regression coverage — please add tests for those alongside the fixes.
The AFD Pareto memory precheck treated AFDConfig.a_batch_size, which is the total in-flight batch per A-Worker, as the per-execution microbatch. Because A-side memory already applies num_microbatches as the KV-cache multiplier, this inflated activation and KV-cache usage by roughly M and rejected feasible fixed-batch candidates as OOM before AFDInferenceSession could run its authoritative memory check. Changes: - pareto_analysis.py: add _afd_total_batch_capacity to convert analytical per-execution microbatch capacity back to total in-flight A batch; apply the conversion to both automatic A-batch derivation and fixed-batch candidate checks, aligning only after conversion to total-batch units; use the same helper for the F-side per-worker capacity conversion. - tests: cover M=1/2/3 capacity conversion and alignment, verify that a fixed A-BS 512 candidate fits a microbatch capacity of 171 with M=3, preserve rejection of a real over-capacity fixed batch, and validate auto-derived total-batch alignment. Signed-off-by: gehujun.ghj <gehujun.ghj@alibaba-inc.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/unit/sdk/test_afd_pareto_review_actions.py`:
- Around line 284-307: Update
test_derive_a_batch_size_aligns_converted_total_capacity to capture the
kvcache_multiplier argument passed to the analytical capacity calculation
instead of discarding it, then assert that the captured value is 3 after calling
_derive_a_batch_size with num_microbatches=3. Keep the existing batch_size
assertion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 403251c5-196c-4fee-8c8e-bce3bc9c4a3b
📒 Files selected for processing (2)
src/aiconfigurator/sdk/pareto_analysis.pytests/unit/sdk/test_afd_pareto_review_actions.py
🚧 Files skipped from review as they are similar to previous changes (1)
- src/aiconfigurator/sdk/pareto_analysis.py
📜 Review details
⏰ Context from checks skipped due to timeout. (6)
- GitHub Check: Collect snapshot (new)
- GitHub Check: Collect snapshot (old)
- GitHub Check: Build and Test (unit)
- GitHub Check: Build and Test (e2e)
- GitHub Check: Cargo Deny
- GitHub Check: Rust/Python engine-step parity
⚠️ CI failures not shown inline (4)
GitHub Actions: Lint and Format / Lint and Format (Ruff): feat(afd): add AFD default CLI mode built on the v2 Task architecture
Conclusion: failure
##[group]Run ruff check .
�[36;1mruff check .�[0m
shell: /usr/bin/bash -e {0}
env:
pythonLocation: /opt/hostedtoolcache/Python/3.12.13/x64
PKG_CONFIG_PATH: /opt/hostedtoolcache/Python/3.12.13/x64/lib/pkgconfig
Python_ROOT_DIR: /opt/hostedtoolcache/Python/3.12.13/x64
Python2_ROOT_DIR: /opt/hostedtoolcache/Python/3.12.13/x64
Python3_ROOT_DIR: /opt/hostedtoolcache/Python/3.12.13/x64
LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.12.13/x64/lib
##[endgroup]
N806 Variable `_BS_LO` in function should be lowercase
--> src/aiconfigurator/sdk/pareto_analysis.py:702:5
|
700 | )
701 |
702 | _BS_LO, _BS_HI = 128, 256
| ^^^^^^
703 | try:
704 | mem_lo = _sample(_BS_LO)
|
N806 Variable `_BS_HI` in function should be lowercase
--> src/aiconfigurator/sdk/pareto_analysis.py:702:13
|
700 | )
701 |
702 | _BS_LO, _BS_HI = 128, 256
| ^^^^^^
703 | try:
704 | mem_lo = _sample(_BS_LO)
|
RUF043 Pattern passed to `match=` contains metacharacters but is neither escaped nor raw
--> tests/unit/sdk/test_afd_pareto.py:281:46
|
280 | def test_prefill_candidate_overflow_errors(self):
281 | with pytest.raises(ValueError, match="afd_config.prefill_search.max_candidates=1"):
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
282 | _enumerate_afd_prefill_options(
283 | model_path="Qwen/Qwen3-32B",
|
help: Use a raw string or `re.escape()` to make the intention explicit
Found 3 errors.
##[error]Process completed with exit code 1.
GitHub Actions: Lint and Format / 0_Lint and Format (Ruff).txt: feat(afd): add AFD default CLI mode built on the v2 Task architecture
Conclusion: failure
##[group]Run ruff check .
�[36;1mruff check .�[0m
shell: /usr/bin/bash -e {0}
env:
pythonLocation: /opt/hostedtoolcache/Python/3.12.13/x64
PKG_CONFIG_PATH: /opt/hostedtoolcache/Python/3.12.13/x64/lib/pkgconfig
Python_ROOT_DIR: /opt/hostedtoolcache/Python/3.12.13/x64
Python2_ROOT_DIR: /opt/hostedtoolcache/Python/3.12.13/x64
Python3_ROOT_DIR: /opt/hostedtoolcache/Python/3.12.13/x64
LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.12.13/x64/lib
##[endgroup]
N806 Variable `_BS_LO` in function should be lowercase
--> src/aiconfigurator/sdk/pareto_analysis.py:702:5
|
700 | )
701 |
702 | _BS_LO, _BS_HI = 128, 256
| ^^^^^^
703 | try:
704 | mem_lo = _sample(_BS_LO)
|
N806 Variable `_BS_HI` in function should be lowercase
--> src/aiconfigurator/sdk/pareto_analysis.py:702:13
|
700 | )
701 |
702 | _BS_LO, _BS_HI = 128, 256
| ^^^^^^
703 | try:
704 | mem_lo = _sample(_BS_LO)
|
RUF043 Pattern passed to `match=` contains metacharacters but is neither escaped nor raw
--> tests/unit/sdk/test_afd_pareto.py:281:46
|
280 | def test_prefill_candidate_overflow_errors(self):
281 | with pytest.raises(ValueError, match="afd_config.prefill_search.max_candidates=1"):
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
282 | _enumerate_afd_prefill_options(
283 | model_path="Qwen/Qwen3-32B",
|
help: Use a raw string or `re.escape()` to make the intention explicit
Found 3 errors.
##[error]Process completed with exit code 1.
GitHub Actions: Copyright Checks / 0_copyright-checks.txt: feat(afd): add AFD default CLI mode built on the v2 Task architecture
Conclusion: failure
f.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/gb300/vllm/0.24.0/mhc_module_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/gb300/vllm/0.24.0/mla_context_module_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/gb300/vllm/0.24.0/mla_generation_module_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/gb300/vllm/0.24.0/moe_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/gb300/vllm/0.24.0/scale_matrix_perf.parquet
[SKIP] src/aiconfigurator/systems/data/h100_pcie/.gitkeep
[WARN] Unsupported: src/aiconfigurator/systems/data/h100_sxm/nccl/2.23/nccl_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/h100_sxm/nccl/2.27.3/nccl_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/h100_sxm/sglang/0.5.10/context_attention_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/h100_sxm/sglang/0.5.10/context_mla_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/h100_sxm/sglang/0.5.10/custom_allreduce_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/h100_sxm/sglang/0.5.10/dsa_context_module_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/h100_sxm/sglang/0.5.10/dsa_generation_module_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/h100_sxm/sglang/0.5.10/encoder_attention_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/h100_sxm/sglang/0.5.10/gdn_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/h100_sxm/sglang/0.5.10/gemm_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/h100_sxm/sglang/0.5.10/generation_attention_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/h100_sxm/sglang/0.5.10/generation_mla_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/h100_sxm/sglang/0.5.10/mla_bmm_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/h100_sxm/sglang/0.5.10/moe...
GitHub Actions: Copyright Checks / copyright-checks: feat(afd): add AFD default CLI mode built on the v2 Task architecture
Conclusion: failure
f.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/gb300/vllm/0.24.0/mhc_module_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/gb300/vllm/0.24.0/mla_context_module_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/gb300/vllm/0.24.0/mla_generation_module_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/gb300/vllm/0.24.0/moe_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/gb300/vllm/0.24.0/scale_matrix_perf.parquet
[SKIP] src/aiconfigurator/systems/data/h100_pcie/.gitkeep
[WARN] Unsupported: src/aiconfigurator/systems/data/h100_sxm/nccl/2.23/nccl_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/h100_sxm/nccl/2.27.3/nccl_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/h100_sxm/sglang/0.5.10/context_attention_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/h100_sxm/sglang/0.5.10/context_mla_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/h100_sxm/sglang/0.5.10/custom_allreduce_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/h100_sxm/sglang/0.5.10/dsa_context_module_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/h100_sxm/sglang/0.5.10/dsa_generation_module_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/h100_sxm/sglang/0.5.10/encoder_attention_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/h100_sxm/sglang/0.5.10/gdn_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/h100_sxm/sglang/0.5.10/gemm_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/h100_sxm/sglang/0.5.10/generation_attention_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/h100_sxm/sglang/0.5.10/generation_mla_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/h100_sxm/sglang/0.5.10/mla_bmm_perf.parquet
[WARN] Unsupported: src/aiconfigurator/systems/data/h100_sxm/sglang/0.5.10/moe...
🧰 Additional context used
📓 Path-based instructions (5)
tests/**/*.{py,yaml,txt,sh}
📄 CodeRabbit inference engine (.claude/rules/generator/testing.md)
Use integration tests for the full input-to-artifacts pipeline, comparing output against golden snapshots without external dependencies.
Files:
tests/unit/sdk/test_afd_pareto_review_actions.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use the project’s
uv-managed environment and run linting withruff check .andruff format --check ..
Files:
tests/unit/sdk/test_afd_pareto_review_actions.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Run unit tests with
pytest -m unit; run the PR build subset withpytest -m "unit or build"when applicable.
Files:
tests/unit/sdk/test_afd_pareto_review_actions.py
**/*
⚙️ CodeRabbit configuration file
**/*: - Prefer applicable inline comments. When the correct fix is clear, small, and limited to the commented diff hunk, include it as a GitHub Suggested Change so the author can apply it with one click.
- Do not use a suggested change when the fix requires broader design choices, multiple files, generated artifacts, unavailable context, or validation that cannot be inferred from the diff.
- If a comment is not directly applicable, state the smallest concrete next step and why a one-click suggestion is not safe.
Files:
tests/unit/sdk/test_afd_pareto_review_actions.py
tests/**
⚙️ CodeRabbit configuration file
tests/**: - Check that tests cover the changed behavior rather than only the happy path.
- Watch for fixtures or golden outputs that mask backend drift, support-matrix ordering changes, or CLI output regressions.
Files:
tests/unit/sdk/test_afd_pareto_review_actions.py
🔇 Additional comments (2)
tests/unit/sdk/test_afd_pareto_review_actions.py (2)
127-143: LGTM!
239-281: LGTM!
| def test_derive_a_batch_size_aligns_converted_total_capacity(monkeypatch): | ||
| monkeypatch.setattr(pa, "get_model", lambda *_args, **_kwargs: object()) | ||
| monkeypatch.setattr( | ||
| "aiconfigurator.sdk.afd_partition.build_afd_ops_partition", | ||
| lambda *_args, **_kwargs: SimpleNamespace(attn_ops=[]), | ||
| ) | ||
| monkeypatch.setattr(pa, "_analytical_max_batch_size", lambda *_args, **_kwargs: 15) | ||
|
|
||
| backend = SimpleNamespace(name=SimpleNamespace(value="trtllm")) | ||
| batch_size, _, _ = pa._derive_a_batch_size( | ||
| "Qwen/Qwen3-32B", | ||
| ModelConfig(), | ||
| backend, | ||
| _FakeDatabase(), | ||
| num_microbatches=3, | ||
| boundary_on_attn=False, | ||
| isl=128, | ||
| osl=32, | ||
| prefix=0, | ||
| max_seq_len=None, | ||
| free_gpu_memory_fraction=None, | ||
| ) | ||
|
|
||
| assert batch_size == 40 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the microbatch multiplier reaches the analytical capacity calculation.
The stub discards kvcache_multiplier, so this test would still pass if _derive_a_batch_size stopped accounting for num_microbatches during KV-cache analysis. Capture the kwargs and assert the value is 3.
As per path instructions, tests must cover the changed behavior rather than only the happy path.
Suggested change
def test_derive_a_batch_size_aligns_converted_total_capacity(monkeypatch):
+ analytical_kwargs = {}
+
monkeypatch.setattr(pa, "get_model", lambda *_args, **_kwargs: object())
monkeypatch.setattr(
"aiconfigurator.sdk.afd_partition.build_afd_ops_partition",
lambda *_args, **_kwargs: SimpleNamespace(attn_ops=[]),
)
- monkeypatch.setattr(pa, "_analytical_max_batch_size", lambda *_args, **_kwargs: 15)
+
+ def fake_analytical_max_batch_size(*_args, **kwargs):
+ analytical_kwargs.update(kwargs)
+ return 15
+
+ monkeypatch.setattr(pa, "_analytical_max_batch_size", fake_analytical_max_batch_size)
...
assert batch_size == 40
+ assert analytical_kwargs["kvcache_multiplier"] == 3📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def test_derive_a_batch_size_aligns_converted_total_capacity(monkeypatch): | |
| monkeypatch.setattr(pa, "get_model", lambda *_args, **_kwargs: object()) | |
| monkeypatch.setattr( | |
| "aiconfigurator.sdk.afd_partition.build_afd_ops_partition", | |
| lambda *_args, **_kwargs: SimpleNamespace(attn_ops=[]), | |
| ) | |
| monkeypatch.setattr(pa, "_analytical_max_batch_size", lambda *_args, **_kwargs: 15) | |
| backend = SimpleNamespace(name=SimpleNamespace(value="trtllm")) | |
| batch_size, _, _ = pa._derive_a_batch_size( | |
| "Qwen/Qwen3-32B", | |
| ModelConfig(), | |
| backend, | |
| _FakeDatabase(), | |
| num_microbatches=3, | |
| boundary_on_attn=False, | |
| isl=128, | |
| osl=32, | |
| prefix=0, | |
| max_seq_len=None, | |
| free_gpu_memory_fraction=None, | |
| ) | |
| assert batch_size == 40 | |
| def test_derive_a_batch_size_aligns_converted_total_capacity(monkeypatch): | |
| analytical_kwargs = {} | |
| monkeypatch.setattr(pa, "get_model", lambda *_args, **_kwargs: object()) | |
| monkeypatch.setattr( | |
| "aiconfigurator.sdk.afd_partition.build_afd_ops_partition", | |
| lambda *_args, **_kwargs: SimpleNamespace(attn_ops=[]), | |
| ) | |
| def fake_analytical_max_batch_size(*_args, **kwargs): | |
| analytical_kwargs.update(kwargs) | |
| return 15 | |
| monkeypatch.setattr(pa, "_analytical_max_batch_size", fake_analytical_max_batch_size) | |
| backend = SimpleNamespace(name=SimpleNamespace(value="trtllm")) | |
| batch_size, _, _ = pa._derive_a_batch_size( | |
| "Qwen/Qwen3-32B", | |
| ModelConfig(), | |
| backend, | |
| _FakeDatabase(), | |
| num_microbatches=3, | |
| boundary_on_attn=False, | |
| isl=128, | |
| osl=32, | |
| prefix=0, | |
| max_seq_len=None, | |
| free_gpu_memory_fraction=None, | |
| ) | |
| assert batch_size == 40 | |
| assert analytical_kwargs["kvcache_multiplier"] == 3 |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/unit/sdk/test_afd_pareto_review_actions.py` around lines 284 - 307,
Update test_derive_a_batch_size_aligns_converted_total_capacity to capture the
kvcache_multiplier argument passed to the analytical capacity calculation
instead of discarding it, then assert that the captured value is 3 after calling
_derive_a_batch_size with num_microbatches=3. Keep the existing batch_size
assertion.
Source: Path instructions
| max_total_bs = _afd_total_batch_capacity(max_micro_bs, kvcache_multiplier, align_to=8) | ||
|
|
||
| return max(min(max_bs, 256), 32), a_model, a_partition | ||
| return max(min(max_total_bs, 256), 32), a_model, a_partition |
There was a problem hiding this comment.
The 256 cap clips exactly the region where AFD wins. At long context, with the (now-correct)
capacity accounting, the optimal total A-pool batch commonly lands in the 300–1000 range —
e.g. Qwen3-235B-A22B isl=32K on 32×H200: the KV-capacity-feasible optimum is ~384 total, and
capping at 256 costs ~20% throughput on the winning configs, enough to flip several
AFD-vs-baseline verdicts near the crossover.
Suggest raising the cap to ≥1024, or making it a searchable/CLI knob. (The floor-to-32
behavior is fine as documented — the caller's per-candidate OOM check handles it.)
| # values. | ||
| if combined_max_bs >= 32: | ||
| bs_align = 8 | ||
| bs_min = 32 |
There was a problem hiding this comment.
With this floor the effective batch grid is {1, 2, 4} ∪ {≥32}: nothing between 5 and 31 is
ever evaluated. At long context that hole covers the feasible optimum — e.g. Qwen3-235B
isl=64K, TPOT≤50ms: per-user KV is multiple GiB, so the best feasible total batch is 16–24.
The search then either returns an infeasible ≥32 point (rejected later) or falls back to the
far-inferior ≤4 point, and the scenario is misreported as "AFD loses" when it's actually near
parity.
Suggest bs_min = 8 (our sweeps converge with that), or extending the low-side grid to
{1, 2, 4, 8, 16, 24}.
| return max(min(max_total_bs, 256), 32), a_model, a_partition | ||
|
|
||
|
|
||
| _AFD_BALANCE_RATIO_THRESHOLD = 0.3 |
There was a problem hiding this comment.
Thanks for lowering this from 0.5 — that removes most of the mis-pruning we had measured.
One residual concern: _quick_balance_ratio probes A and F at the same bs≤32 and doesn't
account for the n_a_workers×M token aggregation on the F side (nor comm), so its ratio can
deviate ~0.2 absolute from the full session — we measured quick=0.92 vs actual=0.72 on
DeepSeek-V3 with a 1:1 node split. At threshold 0.3 the surviving risk is small but nonzero
(deep-MoE models with heavy aggregation).
Two cheap hardening options: (a) log pruned candidates at INFO with their quick-ratio so a
suppressed region is visible in the sweep summary, or (b) make the prune advisory-only and
let the SLA filter do the rejecting.
| num_microbatches, | ||
| pipeline_model, | ||
| ) | ||
| except Exception as e: |
There was a problem hiding this comment.
Candidate failures are only visible at DEBUG. If a whole tier fails systematically (e.g. an
op-name change breaking one tp_a), the search space silently narrows and the sweep still
"succeeds" with worse results. Suggest logging at WARNING (or keeping DEBUG per-candidate but
emitting a per-sweep summary counter: "N/M AFD candidates failed — top reasons: ...").
| effective_per_worker = option["seq_s"] * prefill_degradation | ||
| if effective_per_worker <= 0.0: | ||
| continue | ||
| num_workers = max(1, math.ceil(decode_seq_s * decode_degradation / effective_per_worker)) |
There was a problem hiding this comment.
ceil(decode_rate / per_worker) only ever generates prefill pools that keep up with decode, and the min-GPU selection below then picks the smallest such pool. Prefill-BOUND deployments — fewer prefill workers than rate-match, decode partially idle, system throughput set by prefill — are never generated, yet they win whenever prefill is the scarce resource (e.g. models whose prefill is only feasible at tp8, or short-osl workloads at tight budgets).
Suggested change: enumerate num_workers ∈ [1..ceil(...)], compute the combined row for each, keep the best by end-to-end tokens/s/gpu (min-GPU selection is only optimal under exact rate-matching).
4 participants
bash
aiconfigurator cli default --serving-mode afd
and get a Pareto sweep, SLA-aware picking, and a full report/summary,
end-to-end through the v2 Task stack.
Motivation
configuration handling on top of the old TaskConfig.
last serving mode still using the legacy adapter.
unified sweep interface) and keeps AFD in sync with future Task/sweep
improvements automatically.
Changes
v2 Task (sdk/task_v2.py)
legacy module.
Sweep pipeline (sdk/sweep.py, sdk/pareto_analysis.py)
CLI (cli/main.py, cli/report_and_save.py, cli/utils.py)
default and experiment modes.
Tests
Backward Compatibility
same flags as before.
CLI path; SDK users on the legacy API are unaffected.
Testing
produces a valid AFD Pareto report end-to-end.
Checklist
Summary by CodeRabbit
--serving-mode afdsupport across the default workflow and experiment tasks, including AFD Pareto sweep APIs (sweep_afd/afd_pareto).h20) hardware profile support.