Fix issue when reshape with none edge. #67
Conversation
|
@gausshj 你试试实现对 |
|
@gausshj 你也思考一下，除了这个情况，是否已经覆盖所有的corner case了。 |
- Update the assertation statement for none edge reshape - Fix bugs of reshape caused by new features - Fix typo issues
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
- Fix dimensional issues when reshape with none edges - Add assertion test for none edges reshape
- Fix typo issues of reshape test Signed-off-by: Gausshj <kylin_0@qq.com>
grassmann_tensor/tensor.py
Outdated
| ) | ||
| cursor_self = self.tensor.dim() - 1 | ||
| else: | ||
| if cursor_plan != len(new_shape) and new_shape[cursor_plan] == -1: |
There was a problem hiding this comment.
为什么要加上这个判断 cursor_plan != len(new_shape) ？
There was a problem hiding this comment.
因为对于没有0维度的grassmann张量分解时，如果不加上这个判断会出现问题，cursor_plan这个变量可能会出现边界值。
| # One dimension included, check if we can stop | ||
| if plan_total == self.tensor.shape[cursor_self]: | ||
| # new_shape block has been verified to be always tuple[int, int] before | ||
| if self.tensor.dim() == 0: |
There was a problem hiding this comment.
你在整个流程多次判断self.tensor.dim不如直接放在外面，这样时不时判断一下有点考验可读性。。。
There was a problem hiding this comment.
如果你经过思考，确认确实只有前后dim=0的情况会触发问题，直接在最外面做判断吧。
There was a problem hiding this comment.
但是对于0维度张量的merge和split的逻辑基本是相同的，只是对这些特殊情况进行了处理，如果在最外层判断，可能得把相同的代码逻辑移植到外面，可能会使这个函数更长。
There was a problem hiding this comment.
好，目前这个版本 2a2ab4e 可读性还行。
- Flatten if-else logic for readability Signed-off-by: Gausshj <kylin_0@qq.com>
- Flatten if-else logic for readability Signed-off-by: Gausshj <kylin_0@qq.com>
|
看起来没啥大问题。我这两天再想想有没有啥情况漏的，你也想一想，如果没啥问题，周一合吧。 |
2 participants
No description provided.