Skip to content

(patch) clarify baseline vs effective embed config in logs#1782

Open
jioffe502 wants to merge 1 commit intoNVIDIA:mainfrom
jioffe502:fix/batch-embed-config-logging
Open

(patch) clarify baseline vs effective embed config in logs#1782
jioffe502 wants to merge 1 commit intoNVIDIA:mainfrom
jioffe502:fix/batch-embed-config-logging

Conversation

@jioffe502
Copy link
Copy Markdown
Collaborator

Log BaselineRequestedPlan before overrides and EffectiveEmbedConfig after graph node overrides resolve, so harness/CLI tuning is not mistaken for the heuristic RequestedPlan snapshot.

During testing I noticed that explicit harness/CLI embed overrides did not appear in the initial RequestedPlan log—that snapshot is from cluster heuristics before overrides. The subprocess command was correct, but the logs were misleading.

This PR labels that line as baseline-only and logs the effective embed actor count, batch size, and GPU fraction once overrides are applied.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.
  • If adjusting docker-compose.yaml environment variables have you ensured those are mimicked in the Helm values.yaml file.

Log BaselineRequestedPlan before overrides and EffectiveEmbedConfig after
graph node overrides resolve, so harness/CLI tuning is not mistaken for the
heuristic RequestedPlan snapshot.

Signed-off-by: jioffe502 <jioffe@nvidia.com>
@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot bot commented Apr 2, 2026

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@jioffe502 jioffe502 requested review from a team as code owners April 2, 2026 16:50
@jioffe502 jioffe502 requested a review from nkmcalli April 2, 2026 16:50
Copy link
Copy Markdown
Collaborator

@jperez999 jperez999 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file no longer exists in the code base nor should it come back. Do we need these changes perhaps in graph_ingestor or maybe in the graph_pipeline.py?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants