Repository navigation
fix(planner): unify simulation defaults with PlannerConfig - #15678
Conversation
Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
github-actions
Bot
added
fix
documentation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (12)
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review. WalkthroughPlanner configuration now supplies validated defaults used by prediction, recommendation search, and replay materialization. Recommendation candidates are filtered by target and scaling policy. Tests and documentation cover the updated configuration and CLI behavior. ChangesPlanner configuration flow
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to The change unifies simulation defaults with the production Planner configuration and adds validation, tests and documentation. No concrete merge-blocking issue was identified in the supplied context. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 6.45% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 31 functions across 8 files. (4 skipped: 4 unsupported.)
Comment |
![dynamo-review-agent[bot]](https://avatars.githubusercontent.com/u/201626793?s=60&v=4){kind=small}
Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
There was a problem hiding this comment.
Previously reported defects still present:
- Original discussion: Still present:
_public_planner_configfilters exported public fields withplanner_config.get(runtime) is not None, so valid falsy runtime values such asmin_endpoint: 0are omitted from the recommendation-to-prediction round trip. - Original discussion: Still present: the explicit default scaling-flags parameter remains in
test_prediction_defaults_and_inactive_nulls_match_real_planner, duplicating the empty-overrides coverage rather than protecting distinct behavior. - Original discussion: The explicit default scaling-flags parameter in test_prediction_defaults_and_inactive_nulls_match_real_planner is still present. It exercises the same effective PlannerConfig normalization as the empty-overrides case because non-SLA prediction targets force load scaling, so it cannot distinguish ignored explicit defaults from default handling.
⏹️ Dynamo PR CI cancelled — run 37529060170 (attempt 2) on
|
| Other | Jobs |
|---|---|
| dynamo-runtime | ⏹️ 1 |
| frontend | ⏹️ 1 |
| planner | ⏹️ 1 |
| triton-runtime | ⏹️ 1 |
Failure details
No job failed outright: all 4 cancelled jobs are multi-arch image builds that hit the job timeout-minutes limit during Build and Push Image (GitHub reports this as cancelled). They were not fail-fast or manual cancels; the gate checks fail only because of them.
⏰ dynamo-runtime / image / Build multi-arch cuda13.0: exceeded max execution time of 1h0m0s
Job: dynamo-runtime / image / Build multi-arch cuda13.0 · Step: Build and Push Image · Logs: gh api repos/ai-dynamo/dynamo/actions/jobs/112523306734/logs
##[error]The job has exceeded the maximum execution time of 1h0m0s
##[error]The operation was canceled.
Timed out after 60 min while the multi-arch buildkit build was still running (last amd64 output: wheel_builder 11/11 kvbm NOTICES step done). No build error in the log.
⏰ planner / Build multi-arch cpu: exceeded max execution time of 1h0m0s
Job: planner / Build multi-arch cpu · Step: Build and Push Image · Logs: gh api repos/ai-dynamo/dynamo/actions/jobs/112523305968/logs
##[error]The job has exceeded the maximum execution time of 1h0m0s
##[error]The operation was canceled.
Timed out after 60 min mid-build (last amd64 output: runtime_wheel_builder 11/15 rust license harvest). No build error in the log.
⏰ triton-runtime / Build multi-arch cpu: exceeded max execution time of 1h0m0s
Job: triton-runtime / Build multi-arch cpu · Step: Build and Push Image · Logs: gh api repos/ai-dynamo/dynamo/actions/jobs/112523306056/logs
##[error]The job has exceeded the maximum execution time of 1h0m0s
##[error]The operation was canceled.
Timed out after 60 min mid-build (last output: runtime_wheel_builder 7/13 COPY steps). No build error in the log.
⏰ frontend / Build multi-arch cpu: exceeded max execution time of 1h30m0s
Job: frontend / Build multi-arch cpu · Step: Build and Push Image · Logs: gh api repos/ai-dynamo/dynamo/actions/jobs/112523305858/logs
##[error]The job has exceeded the maximum execution time of 1h30m0s
##[error]The operation was canceled.
Timed out after 90 min mid-build (last amd64 output: pre_frontend 21/22 done). No build error in the log.
All four builds ran slowly and timed out at about the same time, with no error in the build output. That points to slow builds or runners, not a code failure. A rerun of failed jobs is likely enough.
For agents
{"pr": 15678, "run_id": 37529060170, "run_attempt": 2, "head_sha": "cbd17ff49e0f5c3ee4ce5a83b5ab2ac3716217d1",
"failures": [
{"job": "dynamo-runtime / image / Build multi-arch cuda13.0", "job_id": 112523306734, "failed_step": "Build and Push Image", "signature": "job timeout 1h0m0s (cancelled)", "tests": [], "log_cmd": "gh api repos/ai-dynamo/dynamo/actions/jobs/112523306734/logs"},
{"job": "planner / Build multi-arch cpu", "job_id": 112523305968, "failed_step": "Build and Push Image", "signature": "job timeout 1h0m0s (cancelled)", "tests": [], "log_cmd": "gh api repos/ai-dynamo/dynamo/actions/jobs/112523305968/logs"},
{"job": "triton-runtime / Build multi-arch cpu", "job_id": 112523306056, "failed_step": "Build and Push Image", "signature": "job timeout 1h0m0s (cancelled)", "tests": [], "log_cmd": "gh api repos/ai-dynamo/dynamo/actions/jobs/112523306056/logs"},
{"job": "frontend / Build multi-arch cpu", "job_id": 112523305858, "failed_step": "Build and Push Image", "signature": "job timeout 1h30m0s (cancelled)", "tests": [], "log_cmd": "gh api repos/ai-dynamo/dynamo/actions/jobs/112523305858/logs"}
]}Posted automatically by Devin for run 37529060170. Updated on every full-CI run of this PR.
…nner-simulation-config Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
There was a problem hiding this comment.
Previously reported defects still present:
- Original discussion: The Prometheus-environment isolation problem remains on the concrete prediction/materialization path: _planner_config_payload still constructs a full PlannerConfig, but only neutralizes metric_pulling_prometheus_extra_query_params. Other environment-derived fields such as PROMETHEUS_CA_BUNDLE are still read and validated even though the returned hook payload excludes those connection credentials, so a supported simulation prediction or recommendation can still fail because of stale local Prometheus environment. Pass neutral values for the excluded Prometheus connection fields, or otherwise validate only the adapter-owned fields needed for the exported planner_config.
Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
Summary
A minimal
planner: {policy: enabled}prediction was rejected before the production Planner could normalize its default throughput target to load-only scaling. Default recommendation presets also failed when some options were incompatible, and inactive null intervals could not reach the runtime safely.PlannerConfig, removing the duplicate prediction model. Share concrete preset defaults and keep essential predictor/sampling validation in the canonical model.planner.min_num_gpusso exported configurations round-trip without changing Planner limits.Addresses UAT-067 and UAT-068 only in AIC-2023. Other findings in that aggregate ticket remain separate.
Validation
InfeasibleCandidate; validated review findings fixed and rechecked.fern checkhas the same 26 missing generated API-page errors as the unmodified base;fern docs broken-linkscannot initialize because the same generated API index is missing on both.Tests use task-local Python 3.12 and the pinned AISimulate
0.13.0.dev202609300000000061. Native replay uses the existing local UAT build; the relevant replay/mocker Rust sources are unchanged from its base to this PR base. No GPU or Kubernetes run was performed.Summary by CodeRabbit