Skip to content

Conversation

@SigureMo
Copy link
Member

@SigureMo SigureMo commented May 15, 2025

PR Category

Execute Infrastructure

PR Types

Bug fixes

Description

LLM inference 暴露的问题(使用了 fused_rms_norm

SOT 支持 Optional[Tensor] 输出,为确保类型安全,使用 MetaInfoOrNull(i.e. Option<MetaInfo> in rust,MetaInfo | None 在我们如此低程度的类型提示覆盖率下,难以做到完善)

为确保输出和动态图一致是 Tensor(Not initialized),调整了 CreateTensorFromValue 适配 Value(nullptr) case,并移除了 sinking_decomp 里对 Value(nullptr) 转为 None 的特判逻辑

@SigureMo SigureMo requested review from gouzil and zrr1999 as code owners May 15, 2025 10:52
@paddle-bot
Copy link

paddle-bot bot commented May 15, 2025

你的PR提交成功,感谢你对开源项目的贡献!
请关注后续CI自动化测试结果,详情请参考Paddle-CI手册
Your PR has been submitted. Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

@SigureMo SigureMo requested a review from Copilot May 15, 2025 10:52
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a bug in the SOT optional tensor output path by constructing a NullInfo for uninitialized tensors and adapting related functions to use MetaInfoBase. Key changes include adding a new test for optional tensor outputs, updating type checks and function signatures from MetaInfo to MetaInfoBase, and modifying tensor creation logic in the pybind layer.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
test/dygraph_to_static/test_optional_tensor.py Introduces a new test to verify that optional tensor outputs are uninitialized.
python/paddle/jit/sot/opcode_translator/executor/variables/basic.py Refactors type annotations and stringified guard logic to reference MetaInfoBase.
python/paddle/jit/sot/opcode_translator/executor/function_graph.py Updates predicate type checks to use MetaInfoBase.
python/paddle/jit/sot/infer_meta.py Abstracts MetaInfo into MetaInfoBase and adds NullInfo handling for uninitialized tensors.
paddle/fluid/pybind/pybind.cc Removes manual py::list construction in favor of directly returning a vector of pir::Value.
paddle/fluid/pybind/eager_utils.cc Adjusts tensor creation to set autograd meta before handling nullable Value cases.
Comments suppressed due to low confidence (2)

python/paddle/jit/sot/infer_meta.py:911

  • get_symbolic_from_meta() enforces that meta is an instance of MetaInfo (via an isinstance check), which may be too restrictive when NullInfo (a valid MetaInfoBase subclass) is passed for optional tensor outputs. Consider updating the type check or handling for NullInfo to avoid unexpected errors.
def get_symbolic_from_meta(meta: MetaInfoBase) -> SymbolicValue: 

paddle/fluid/pybind/pybind.cc:1042

  • Changing the return value from a py::list to a std::vectorpir::Value might affect downstream code that expects a Python list interface. Ensure that consumers of this API are updated to handle the new return type.
return tar_vars; 
gouzil
gouzil previously approved these changes May 15, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a bug in LLM inference by enabling support for optional tensor outputs within the SOT framework. Key changes include adding tests for optional tensor outputs, introducing a new exception class (NullMetaBreak), and updating type annotations and logic across the SOT modules to handle MetaInfoOrNull.

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
test/dygraph_to_static/test_optional_tensor.py Adds a new test for verifying optional tensor output behavior.
python/paddle/jit/sot/utils/exceptions.py Introduces the NullMetaBreak exception for handling null meta cases.
python/paddle/jit/sot/symbolic/compile_cache.py Updates type annotations for input_spec to support Optional[InputSpec].
python/paddle/jit/sot/symbolic/builder.py Updates signature and guard implementations to work with MetaInfoOrNull.
python/paddle/jit/sot/opcode_translator/executor/variables/basic.py Refactors variable handling to replace MetaInfo with MetaInfoOrNull.
python/paddle/jit/sot/opcode_translator/executor/function_graph.py Modifies variable retrieval and meta handling to use MetaInfoOrNull.
python/paddle/jit/sot/infer_meta.py Refactors MetaInfo management to support optional meta outputs with MetaInfoOrNull.
paddle/fluid/pybind/pybind.cc Simplifies sinking decomp rule output generation.
paddle/fluid/pybind/eager_utils.cc Adjusts CreateTensorFromValue to correctly handle Value(nullptr) cases.
Comments suppressed due to low confidence (1)

python/paddle/jit/sot/symbolic/builder.py:522

  • The branch for handling uninitialized tensors currently returns an empty guard list with a TODO comment. To ensure reliable guard checking, consider implementing a proper guard or adding clear documentation on how this case should be handled in production.
if meta.is_null(): return [ # TODO: Implement uninitialized tensor guard ] 
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 81.50685% with 27 lines in your changes missing coverage. Please review.

Please upload report for BASE (develop@dc813e5). Learn more about missing BASE report.

Files with missing lines Patch % Lines
python/paddle/jit/sot/infer_meta.py 85.86% 13 Missing ⚠️
.../sot/opcode_translator/executor/variables/basic.py 73.46% 13 Missing ⚠️
python/paddle/jit/sot/utils/exceptions.py 66.66% 1 Missing ⚠️

❌ Your patch status has failed because the patch coverage (81.50%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@ Coverage Diff @@ ## develop #72737 +/- ## ========================================== Coverage ? 81.50% ========================================== Files ? 4 Lines ? 146 Branches ? 0 ========================================== Hits ? 119 Misses ? 27 Partials ? 0 

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
@SigureMo SigureMo changed the title [SOT][PIR] Support optional tensor output [SOT][PIR][3.13] Support optional tensor output May 16, 2025
@SigureMo SigureMo requested a review from gouzil May 16, 2025 13:26
@SigureMo SigureMo changed the title [SOT][PIR][3.13] Support optional tensor output [SOT][PIR] Support optional tensor output May 16, 2025
@SigureMo SigureMo merged commit 28eae79 into PaddlePaddle:develop May 16, 2025
51 of 52 checks passed
@SigureMo SigureMo deleted the sot/support-optional-tensor-output branch May 16, 2025 14:09
meta = self.origin_meta
if meta.is_null():
return [
# TODO: Implement uninitialized tensor guard
Copy link
Member Author

Choose a reason for hiding this comment

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

@gouzil 川子 [doge],和 591 行对齐就可以了

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

Labels

None yet

3 participants