Skip to content

Conversation

@zzzzwwjj
Copy link
Collaborator

@zzzzwwjj zzzzwwjj commented Dec 1, 2025

What this PR does / why we need it?

Set profiler's param aic_metrics to torch_npu.profiler.AiCMetrics.PipeUtilization by default.

This parameter can obtain more information for op.

Does this PR introduce any user-facing change?

How was this patch tested?

Signed-off-by: zzzzwwjj <1183291235@qq.com>
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the default aic_metrics for the torch NPU profiler to PipeUtilization from AiCoreNone. While enabling more detailed metrics by default is useful for profiling, hardcoding this value could cause issues on hardware where it's not supported or for users who wish to minimize profiling overhead. I have added a comment suggesting to make this parameter configurable via an environment variable to increase flexibility and robustness.

profiler_level=torch_npu.profiler.ProfilerLevel.Level1,
msprof_tx=False,
aic_metrics=torch_npu.profiler.AiCMetrics.AiCoreNone,
aic_metrics=torch_npu.profiler.AiCMetrics.PipeUtilization,
Copy link
Contributor

Choose a reason for hiding this comment

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

high

Hardcoding aic_metrics to PipeUtilization could be problematic. This metric might not be supported on all hardware versions, or some users might prefer to disable it to minimize profiling overhead. It would be more robust to make this configurable via an environment variable (e.g., VLLM_ASCEND_PROFILER_AIC_METRICS defined in vllm_ascend/envs.py). This would allow users to easily switch between different metrics like PipeUtilization and AiCoreNone based on their needs.

@github-actions
Copy link

github-actions bot commented Dec 1, 2025

👋 Hi! Thank you for contributing to the vLLM Ascend project. The following points will speed up your PR merge:‌‌

  • A PR should do only one thing, smaller PRs enable faster reviews.
  • Every PR should include unit tests and end-to-end tests ‌to ensure it works and is not broken by other future PRs.
  • Write the commit message by fulfilling the PR description to help reviewer and future developers understand.

If CI fails, you can run linting and testing checks locally according Contributing and Testing.

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.

1 participant