-
Notifications
You must be signed in to change notification settings - Fork 0
Fix corner cases in reshape and improve its robustness #72
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
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -221,6 +221,20 @@ def _reorder_indices( | |
| sign = (count & 2).to(dtype=torch.bool) | ||
| return len(even), len(odd), reorder, sign.flatten() | ||
|
|
||
| def _calculate_even_odd(self) -> tuple[int, int]: | ||
| return self.calculate_even_odd(self.edges) | ||
|
|
||
| @staticmethod | ||
| def calculate_even_odd(edges: tuple[tuple[int, int], ...]) -> tuple[int, int]: | ||
| return functools.reduce( | ||
| lambda accumulator, even_odd_pair: ( | ||
| accumulator[0] * even_odd_pair[0] + accumulator[1] * even_odd_pair[1], | ||
| accumulator[0] * even_odd_pair[1] + accumulator[1] * even_odd_pair[0], | ||
| ), | ||
| edges, | ||
| (1, 0), | ||
| ) | ||
|
|
||
| def reshape(self, new_shape: tuple[int | tuple[int, int], ...]) -> GrassmannTensor: | ||
| """ | ||
| Reshape the Grassmann tensor, which may split or merge edges. | ||
|
|
@@ -253,15 +267,38 @@ def reshape(self, new_shape: tuple[int | tuple[int, int], ...]) -> GrassmannTens | |
| merging_reorder: list[tuple[int, torch.Tensor]] = [] | ||
| merging_sign: list[tuple[int, torch.Tensor]] = [] | ||
|
|
||
| original_self_is_scalar = self.tensor.dim() == 0 | ||
| if original_self_is_scalar: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 把这个判断放最外面挺好的,不过为啥只有original是scalar的判断,没有target是scalar的判断?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 放在外面确实好很多,现在已经在607dfd8把target判断放在了外面。代码复杂度和可读性大大提高了。 |
||
| new_shape_list: list[tuple[int, int]] = [] | ||
| for item in new_shape: | ||
| if item == -1: | ||
| raise AssertionError("Cannot use -1 when reshaping from a scalar") | ||
| if isinstance(item, int): | ||
| if item != 1: | ||
| raise AssertionError( | ||
| f"Ambiguous integer dim {item} from scalar. " | ||
| "Use explicit (even, odd) pairs, or only use 1 for trivial edges." | ||
| ) | ||
| new_shape_list.append((1, 0)) | ||
| else: | ||
| new_shape_list.append(item) | ||
| new_shape = tuple(new_shape_list) | ||
| edges_only = typing.cast(tuple[tuple[int, int], ...], new_shape) | ||
| assert self.calculate_even_odd(edges_only) == (1, 0), ( | ||
| "Cannot split none edges into illegal edges" | ||
| ) | ||
|
|
||
| if len(new_shape) == 0: | ||
| assert self._calculate_even_odd() == (1, 0), ( | ||
| "Only pure even edges can be merged into none edges" | ||
| ) | ||
| tensor = self.tensor.reshape(()) | ||
| return GrassmannTensor(_arrow=(), _edges=(), _tensor=tensor) | ||
|
|
||
| cursor_plan: int = 0 | ||
| cursor_self: int = 0 | ||
| while cursor_plan != len(new_shape) or cursor_self != self.tensor.dim(): | ||
| if len(new_shape) == 0: | ||
| assert all(edge == (0, 1) or edge == (1, 0) for edge in self.edges), ( | ||
| f"Edge must be (0, 1) or (1, 0) but got {self.edges}" | ||
| ) | ||
| cursor_self = self.tensor.dim() - 1 | ||
| elif cursor_plan != len(new_shape) and new_shape[cursor_plan] == -1: | ||
| if cursor_plan != len(new_shape) and new_shape[cursor_plan] == -1: | ||
| # Does not change | ||
| arrow.append(self.arrow[cursor_self]) | ||
| edges.append(self.edges[cursor_self]) | ||
|
|
@@ -280,20 +317,14 @@ def reshape(self, new_shape: tuple[int | tuple[int, int], ...]) -> GrassmannTens | |
| # A trivial self edge | ||
| cursor_self += 1 | ||
| continue | ||
| if len(new_shape) == 0: | ||
| cursor_new_shape = typing.cast(int | tuple[int, int], tuple()) | ||
| total = 1 | ||
| else: | ||
| cursor_new_shape = new_shape[cursor_plan] | ||
| total = ( | ||
| cursor_new_shape | ||
| if isinstance(cursor_new_shape, int) | ||
| else cursor_new_shape[0] + cursor_new_shape[1] | ||
| ) | ||
| cursor_new_shape = new_shape[cursor_plan] | ||
| total = ( | ||
| cursor_new_shape | ||
| if isinstance(cursor_new_shape, int) | ||
| else cursor_new_shape[0] + cursor_new_shape[1] | ||
| ) | ||
| # one of total and shape[cursor_self] is not trivial, otherwise it should be handled before | ||
| if self.tensor.dim() == 0: | ||
| merging = False | ||
| elif total == self.tensor.shape[cursor_self]: | ||
| if total == self.tensor.shape[cursor_self]: | ||
| # We do not know whether it is merging or splitting, check more | ||
| if isinstance(cursor_new_shape, int) or cursor_new_shape == self.edges[cursor_self]: | ||
| # If the new shape is exactly the same as the current edge, we treat it as no change | ||
|
|
@@ -307,9 +338,6 @@ def reshape(self, new_shape: tuple[int | tuple[int, int], ...]) -> GrassmannTens | |
| cursor_self_finding = cursor_self | ||
| cursor_self_found = False | ||
| while True: | ||
| if len(new_shape) == 0: | ||
| cursor_self_found = True | ||
| break | ||
| cursor_self_finding += 1 | ||
| if cursor_self_finding == self.tensor.dim(): | ||
| break | ||
|
|
@@ -329,10 +357,6 @@ def reshape(self, new_shape: tuple[int | tuple[int, int], ...]) -> GrassmannTens | |
| new_cursor_self = cursor_self | ||
| self_total = 1 | ||
| while True: | ||
| if len(new_shape) == 0: | ||
| new_cursor_self += 1 | ||
| even, odd, reorder, sign = self._reorder_indices(self.edges) | ||
| break | ||
| # Try to include more dimension from self | ||
| self_total *= self.tensor.shape[new_cursor_self] | ||
| new_cursor_self += 1 | ||
|
|
@@ -354,26 +378,19 @@ def reshape(self, new_shape: tuple[int | tuple[int, int], ...]) -> GrassmannTens | |
| f"New shape exceeds in merging with edges {self.edges} and new shape {new_shape}." | ||
| ) | ||
| # The merging block [cursor_self, new_cursor_self) has been determined | ||
| if len(new_shape) == 0: | ||
| arrow = [] | ||
| edges = [] | ||
| shape = [] | ||
| merging_sign.append((cursor_plan, sign)) | ||
| cursor_self = new_cursor_self | ||
| else: | ||
| arrow.append(self.arrow[cursor_self]) | ||
| assert all( | ||
| self_arrow == arrow[-1] | ||
| for self_arrow in self.arrow[cursor_self:new_cursor_self] | ||
| ), ( | ||
| f"Cannot merge edges with different arrows {self.arrow[cursor_self:new_cursor_self]}." | ||
| ) | ||
| edges.append((even, odd)) | ||
| shape.append(total) | ||
| merging_sign.append((cursor_plan, sign)) | ||
| merging_reorder.append((cursor_plan, reorder)) | ||
| cursor_self = new_cursor_self | ||
| cursor_plan += 1 | ||
| arrow.append(self.arrow[cursor_self]) | ||
| assert all( | ||
| self_arrow == arrow[-1] | ||
| for self_arrow in self.arrow[cursor_self:new_cursor_self] | ||
| ), ( | ||
| f"Cannot merge edges with different arrows {self.arrow[cursor_self:new_cursor_self]}." | ||
| ) | ||
| edges.append((even, odd)) | ||
| shape.append(total) | ||
| merging_sign.append((cursor_plan, sign)) | ||
| merging_reorder.append((cursor_plan, reorder)) | ||
| cursor_self = new_cursor_self | ||
| cursor_plan += 1 | ||
| else: | ||
| # Splitting between [cursor_plan, new_cursor_plan) and the another side contains dimension as plan_total | ||
| new_cursor_plan = cursor_plan | ||
|
|
@@ -387,23 +404,16 @@ def reshape(self, new_shape: tuple[int | tuple[int, int], ...]) -> GrassmannTens | |
| plan_total *= new_cursor_new_shape[0] + new_cursor_new_shape[1] | ||
| new_cursor_plan += 1 | ||
| # One dimension included, check if we can stop | ||
| if self.tensor.dim() == 0: | ||
| if plan_total == self.tensor.shape[cursor_self]: | ||
| # new_shape block has been verified to be always tuple[int, int] before | ||
| even, odd, reorder, sign = self._reorder_indices( | ||
| typing.cast(tuple[tuple[int, int], ...], new_shape) | ||
| ) | ||
| new_cursor_plan = len(new_shape) | ||
| break | ||
| else: | ||
| if plan_total == self.tensor.shape[cursor_self]: | ||
| # new_shape block has been verified to be always tuple[int, int] before | ||
| even, odd, reorder, sign = self._reorder_indices( | ||
| typing.cast( | ||
| tuple[tuple[int, int], ...], | ||
| new_shape[cursor_plan:new_cursor_plan], | ||
| ) | ||
| typing.cast( | ||
| tuple[tuple[int, int], ...], | ||
| new_shape[cursor_plan:new_cursor_plan], | ||
| ) | ||
| if (even, odd) == self.edges[cursor_self]: | ||
| break | ||
| ) | ||
| if (even, odd) == self.edges[cursor_self]: | ||
| break | ||
| # For some reason we cannot stop here, continue to include more dimension, check something before continue | ||
| assert plan_total <= self.tensor.shape[cursor_self], ( | ||
| f"Dimension mismatch in splitting with edges {self.edges} and new shape {new_shape}." | ||
|
|
@@ -415,10 +425,7 @@ def reshape(self, new_shape: tuple[int | tuple[int, int], ...]) -> GrassmannTens | |
| for i in range(cursor_plan, new_cursor_plan): | ||
| # new_shape block has been verified to be always tuple[int, int] in the loop | ||
| new_cursor_new_shape = typing.cast(tuple[int, int], new_shape[i]) | ||
| if self.tensor.dim() == 0: | ||
| arrow.append(False) | ||
| else: | ||
| arrow.append(self.arrow[cursor_self]) | ||
| arrow.append(self.arrow[cursor_self]) | ||
| edges.append(new_cursor_new_shape) | ||
| shape.append(new_cursor_new_shape[0] + new_cursor_new_shape[1]) | ||
| splitting_reorder.append((cursor_self, reorder)) | ||
|
|
@@ -447,9 +454,6 @@ def reshape(self, new_shape: tuple[int | tuple[int, int], ...]) -> GrassmannTens | |
|
|
||
| tensor = tensor.reshape(shape) | ||
|
|
||
| if len(new_shape) == 0: | ||
| return GrassmannTensor(_arrow=tuple(arrow), _edges=tuple(edges), _tensor=tensor) | ||
|
|
||
| merging_parity = functools.reduce( | ||
| torch.logical_xor, | ||
| ( | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
好的,已经在607dfd8中提交了修改。
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.
这个函数原则上可以被上面那个reorder_indices覆盖功能, 但是你在结果是(1, 0)的情况下, 希望做简单/快速的判断. 所以我建议: 直接写一个 is_xxx_valid 之类的函数, 或者直接判断 edges_only.count((0, 1)) % 2 == 0 .
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.
这里这个函数后面开发会不会也有用?我在svd的测试中也用到了类似的函数,这里实现了后续可以直接调用。