-
Notifications
You must be signed in to change notification settings - Fork 56
Compatibility with MultiTensorKit #247
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
Changes from 28 commits
fb4693f
13bcf82
09467c6
2878e08
10744b2
1ed1ed1
c31b99c
a33d028
0e3c5df
124def4
9635362
4bc1e7c
24be2ea
13af986
095dcda
fbfb564
d7cae4f
a14fc70
4fe82f9
55a00c0
0113f73
8e8b760
78b135e
440b42b
00825af
aad8fc8
c0ef01e
c189284
bdbd2e6
e23e092
8ec577e
d915273
301780a
f9cede3
baf1824
9d543ee
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
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. For these changes, I think it would be nice to have a dedicated discussion (indeed in a separate PR) and some comments explaining what is going on. The bottom line being that we want the The implementation below is somewhat of a hack since normally There are a couple options for fixing this:
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. Is this something that came up specifically in this PR? This seems like it should already have been an issue with existing complex sectors.
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. Is this something that came up specifically in this PR? This seems like it should already have been an issue with existing complex sectors.
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. Yes, but I don't think we ever used complex sectors with non-complex tensors all that much, and MPSKit actually uses complex entries for almost everything by default. |
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.
Thanks for catching this. Did you encounter this by reading the code or by an actual case where the old implementation errored? Clearly there should have been a test in place to cover those lines of code. So if you have some example code that actually triggers these lines, that might be a good starting point to extract a test case from (which might be easier then engineering one from scratch).
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.
I found this from an error. However, I cannot find what I was doing to trigger this particular iteration... I will keep looking