| CARVIEW |
Navigation Menu
-
Notifications
You must be signed in to change notification settings - Fork 5.8k
[SOT] fix bug in llm stable diffusion #62257
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
Conversation
|
你的PR提交成功，感谢你对开源项目的贡献! |
| @@ -82,7 +82,6 @@ def get_paddle_api(): | |||
| # considered as paddle module？ | |||
| paddle_api_module_prefix = { | |||
| "paddle.nn.functional", | |||
| "paddle.nn.layer.activation", | |||
| } | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这整个以及 in_paddle_module 应该都可以废弃了吧
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
好滴
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个还是放着把，后面可以加
| @@ -83,7 +82,8 @@ def analysis_used_names( | |||
| Returns: | |||
| set[str]: The analysis result. | |||
| """ | |||
| root_state = State(OrderedSet(), OrderedSet(), OrderedSet()) | |||
| root_state = State(OrderedSet(), OrderedSet()) | |||
| visited = OrderedSet() | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
visited 不可以共享，不然多个分支之间结果一定会相互干涉，结果是不正确的，虽然 CI 过了，但显然是有问题的
可以考虑为 walk 加 cache，如果 walk 输入的 state.reads、state.writes 以及 start 相同，结果一定相同
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
多个分支是会互相干涉，但是它还是正确的，为啥有问题呢
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
因为不同初始状态分析下结果是不一致的
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
只要看 writes 就行了吧，
| @@ -186,6 +186,11 @@ def call_function(self, /, *args, **kwargs) -> VariableBase: | |||
| if output is not None: | |||
| return output | |||
|
|
|||
| if inspect.isgeneratorfunction(self.value): | |||
| raise BreakGraphError( | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
加个单测吧
| root_state = State(OrderedSet(), OrderedSet()) | ||
| visited = {} | ||
|
|
||
| def cache_check_and_update_cache(idx, writes): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
不将 reads 作为 cache key 的话
branch 1: reads={a, b} -- walk --> reads={a, b, c} # +c
branch 2: reads={a, d} -- walk --> reads={a, c, d} # +c
merge: {a, b, c} | {a, c, d} = {a, b, c, d}
如果只考虑 writes，那么 branch2 的结果也是 {a, b, c} 最终两者归并是 {a, b, c}
如果想要只考虑 writes，可以考虑 cache diff（也就是 c），否则会有结果错误的风险
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
名称上加上「剪枝」相关字样吧，这应该是剪枝而不是 cache
| @@ -83,7 +82,26 @@ def analysis_used_names( | |||
| Returns: | |||
| set[str]: The analysis result. | |||
| """ | |||
| root_state = State(OrderedSet(), OrderedSet(), OrderedSet()) | |||
| root_state = State(OrderedSet(), OrderedSet()) | |||
| visited = {} | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这种语义下，不再是 visited，应该就叫 cache
| return True | ||
| elif record.issubset(writes): | ||
| history.remove(record) | ||
| history.append(writes) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这里 writes 扩张，结果里的 writes 为什么不扩张呢？全都依赖于「合并」么？
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个是记录的已经模拟过的state，和输出无关呢
PR types
Others
PR changes
Others
Description
fix bug in llm stable diffusion
PCard-66972