【PIR OpTest Fix No.13】 Fix test_partial_concat_op #62833
Conversation
|
你的PR提交成功，感谢你对开源项目的贡献! |
| if (length > 0) { | ||
| PADDLE_ENFORCE_GE(input_len, | ||
| start_index + length, | ||
| phi::errors::OutOfRange( | ||
| "start_index + length is larger than input length")); | ||
| } |
There was a problem hiding this comment.
为什么多出这部分判断？
check了一下，确实不需要，上面的Enforce已经覆盖了这个情况，已修改
There was a problem hiding this comment.
start_index + length <= input_len 被哪些条件覆盖了，坦白讲我没看出来。这里其实是可以合入的，只是得下补充解释。
There was a problem hiding this comment.
另外PR等CI过了我就直接合入了，你先不要改，这里的改动你加点解释，如果有必要的话可以再提个PR补充。
There was a problem hiding this comment.
start_index
我原本以为上面的start_index >= -input_len && start_index < input_len,这个判断是判断start_index越界的，然后代入到下面start_index + length这个越界判断，已经覆盖了这个情况，刚意识到start_index + length应该等价于end_index，确实需要判断一下
There was a problem hiding this comment.
start_index + length <= input_len 被哪些条件覆盖了，坦白讲我没看出来。这里其实是可以合入的，只是得下补充解释。
我想了一下，按理说，这段的逻辑应该也要加上这个判断，就比如说：
partial_len 如果大于0的话就直推出了partial_len * inputs_num这么多的大小，并没有考虑start_index + length <= input_len 这个条件
There was a problem hiding this comment.
这个条件
也就是越界的话就是推大了，然后我看了一眼它的compute定义，似乎也没有考虑这个，就是这里：
越界了memcpy可能拷贝到下一行的数据了
|
需要解决下冲突 |
已修复 |
PR types
Others
PR changes
Others
Description
PIR Op单测修复
修复单测 test_partial_concat_op
修复后打开FLAGS_enable_pir_in_executor单测是否通过：是