Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Dy2St] Add place hash to scope cache key to avoid conflict with executor cache #71505

Conversation

SigureMo
Copy link
Member

@SigureMo SigureMo commented Mar 8, 2025

PR Category

Execute Infrastructure

PR Types

Bug fixes

Description

我们现在执行器 cache key 同时包含了输入 place 信息和 program id,但是 scope 的 cache key 却只有 program id,这就导致了在 place 发生改变时,program 一样,scope 能复用,但执行器不是同一个,不同的执行器上下文对同一个 scope 进行了修改,最终导致出了奇怪的问题(tensor 突然就没了)

本 PR 只是发版前的临时解决方案,python 端和 C++ 端的 hash 计算不能保证完全一致,后续可能会考虑将 scope cache 移到 C++ 以使两者完全匹配

@SigureMo SigureMo requested review from gouzil and Copilot March 8, 2025 15:42
Copy link

paddle-bot bot commented Mar 8, 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.

Choose a reason for hiding this comment

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

PR Overview

This PR fixes a cache key conflict between the executor and scope by incorporating a place hash into the scope cache key. Key changes include:

  • Introducing a new hash_with_seed function and a _calc_input_places_hash helper.
  • Updating scope creation functions (_get_scope and _create_scope_vec) to use a new cache_key parameter.
  • Adjusting calls in the run_program functions to compute and pass the proper cache_key.

Reviewed Changes

File Description
python/paddle/jit/dy2static/pir_partial_program.py Changes to incorporate a place hash into the scope cache key to avoid conflicts with the executor cache

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

Comments suppressed due to low confidence (2)

python/paddle/jit/dy2static/pir_partial_program.py:54

  • [nitpick] Consider adding type hints and a brief docstring for hash_with_seed to clarify the expected types of 'value' and 'seed'.
def hash_with_seed(value, seed):

python/paddle/jit/dy2static/pir_partial_program.py:1196

  • Update the function documentation for _create_scope_vec to reflect the change from 'program_id' to 'cache_key' for clarity.
def _create_scope_vec(self, cache_key=None, use_scope_cache=False):
@SigureMo SigureMo changed the title [Dy2St] Add place hash to scope cache key to avoid conflict with executor cache [Dy2St][3.13] Add place hash to scope cache key to avoid conflict with executor cache Mar 8, 2025
@SigureMo SigureMo changed the title [Dy2St][3.13] Add place hash to scope cache key to avoid conflict with executor cache [Dy2St] Add place hash to scope cache key to avoid conflict with executor cache Mar 9, 2025
@SigureMo SigureMo merged commit bcfa081 into PaddlePaddle:develop Mar 9, 2025
32 of 33 checks passed
@SigureMo SigureMo deleted the dy2st/add-place-hash-to-scope-cache-key-to-avoid-conflict-with-interpreter branch March 9, 2025 01:48
SigureMo added a commit to cattidea/Paddle that referenced this pull request Mar 9, 2025
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