fix(aic): adopt published AIC 0.11 dev artifacts - #11986
Conversation
Signed-off-by: Jason Zhou (Engrg-Hardware 1) <jasonzho@nvidia.com>
Signed-off-by: Jason Zhou (Engrg-Hardware 1) <jasonzho@nvidia.com>
Signed-off-by: Jason Zhou (Engrg-Hardware 1) <jasonzho@nvidia.com>
Signed-off-by: Jason Zhou (Engrg-Hardware 1) <jasonzho@nvidia.com>
Signed-off-by: Jason Zhou (Engrg-Hardware 1) <jasonzho@nvidia.com>
WalkthroughChangesThe AIC speculative-decoding configuration now uses scalar Speculative acceptance configuration
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@lib/bindings/python/rust/llm/aic_callback.rs`:
- Around line 116-128: Update the nextn conversion in the callback logic to use
u32::try_from instead of narrowing with as u32, and return a PyValueError when
the usize value exceeds u32::MAX. Perform this validation before the nextn > 0
check and before calling _nextn_accepted_from_accept_rates, while preserving the
existing zero and valid-value behavior.
In `@lib/bindings/python/tests/test_aic_capacity.py`:
- Around line 255-258: Update the pytest.raises match in
test_nextn_accepted_rejects_unsupported_mocker_depth to use a raw regex with
both literal dots escaped, ensuring the assertion validates the exact “nextn
must be 1..=5” message.
In `@lib/mocker/src/common/engine_perf.rs`:
- Around line 1136-1144: Update the speculative configuration construction in
the args-to-config flow to ignore aic_nextn values of zero before creating
SpeculativeConfig. Preserve the existing conversion and error propagation for
positive values, ensuring aic_nextn=0 produces no speculative config and matches
into_aic_config’s dense-mode behavior.
🪄 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: b82d3afb-55e0-4487-a823-3e4278e26fb1
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.locklib/bindings/python/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
Cargo.tomlcomponents/src/dynamo/planner/core/perf_model/rust_adapter.pycomponents/src/dynamo/planner/tests/unit/test_rust_perf_adapter.pylib/bindings/python/Cargo.tomllib/bindings/python/rust/llm/aic_callback.rslib/bindings/python/src/dynamo/_internal/aic.pylib/bindings/python/tests/test_aic_capacity.pylib/mocker/src/common/engine_perf.rs
Replacing this request-changes review with a neutral review containing both notes inline.
PeaBrane
left a comment
There was a problem hiding this comment.
Two inline notes: the Python/Rust dependency alignment is the actionable contract issue; the FPM note only requests a TODO documenting a dormant integration boundary.
…n-accepted Signed-off-by: Jason Zhou (Engrg-Hardware 1) <jasonzho@nvidia.com>
Signed-off-by: Jason Zhou (Engrg-Hardware 1) <jasonzho@nvidia.com>
Signed-off-by: Jason Zhou (Engrg-Hardware 1) <jasonzho@nvidia.com>
Signed-off-by: Jason Zhou (Engrg-Hardware 1) <jasonzho@nvidia.com>
Signed-off-by: Jason Zhou (Engrg-Hardware 1) <jasonzho@nvidia.com>
… into agent/update-aic-nextn-accepted
|
/ok to test d19f515 |
Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
|
Rechecked the exact current head The two blocking regressions found against the prior
Focused validation on this head:
I also replied to and resolved the outdated benchmark dependency thread: AIC core is pinned to |
|
/ok to test 70e3176 |
70e3176 to
3d0dbf5
Compare
jthomson04
left a comment
There was a problem hiding this comment.
approving the one frontend change.
Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
…-mypy Signed-off-by: hongkuanz <hongkuanz@nvidia.com> # Conflicts: # container/templates/planner.Dockerfile
saturley-hall
left a comment
There was a problem hiding this comment.
confirmed that the planner container installed the dev version of aiconfigurator-core from the compliance CSV
Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
0ab569d to
635e023
Compare
Summary
aiconfigurator==0.11.0.dev20260728andaiconfigurator-core==0.11.0.dev20260728aiconfigurator-core==0.11.0-dev20260728, with registry checksums recorded in both lockfilesaiconfigurator_corefor Runtime, Mocker, Router, and Planner modeling while retaining upperaiconfiguratorfor Profiler and AI Simulate orchestration/generator surfacesAicEngineBuilderAPI and remove pre-0.11 compatibility/fallback pathsnextnto AIC core for MTP verification cost_execute_taskssix-value return contract, including per-task outcomesPackaging
The Python dev releases are index-resolvable from PyPI. Upper
aiconfiguratorpins the exact matchingaiconfigurator-corerelease; AI Simulate declares both because it directly imports both package layers. The core package uses its published wheel-stub flow to install the platform wheel from the NVIDIA package index.The Rust crate is published and non-yanked on crates.io. Dynamo no longer embeds an AIC Git revision or builds AIC from source in its package metadata or container flow. The Planner image installs the standalone
aisimulatewheel together with the paired AIC packages.Reviewer guide
Cargo.toml, both Cargo lockfiles, root/benchmark/AI Simulate Python metadata, andcontainer/deps/requirements.planner.txtlib/bindings/python/rust/llm/aic_callback.rs,dynamo._internal.aic, Mocker perf-model configuration, replay, and scheduler adaptersValidation
cargo check --locked -p dynamo-mocker --features aic-forward-passcargo check --locked --manifest-path lib/bindings/python/Cargo.toml --features aic-forward-passcargo fmt --manifest-path lib/bindings/python/Cargo.toml -- --checkgit diff --checkFull PR CI is running on the current signed head. Merge remains gated on the required current-head reviews.