Repository navigation
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests.
🚀 New features to boost your workflow:
|
lkdvos
left a comment
There was a problem hiding this comment.
Trying to unpack this here, mostly to see if I'm correctly understanding the goals etc.
Am I correct in saying that this is effectively doing two things:
- exposing
Module._symmetrize_operator - accepting
AbstractArraydirectly
For the first part, sounds good, although I might advocate to keep the name to be symmetrize_operator instead of custom, specifically for an exported function, and just making that exported.
I'm slightly less certain about the second part, as I guess that is more of a convenience layer on top and I tend to like steering people slightly into TensorKit, since realistically they will have to at some point anyways. If you really think this is necessary, I'd just use dispatch and define a small helper function that does TensorMap(A, *(ComplexSpace.(size.(Ref(A), 1:div(ndims(A), 2))...), *(ComplexSpace.(size.(Ref(A), div(ndims(A), 2)+1:ndims(A))...)), so both versions are supported?
This is also more or less why I don't really see much benefit in asserting equal sizes of the input array, as we probably error right after that in the symmetrization code anyways?
Otherwise definitely looks good, thanks for the addition!
|
Now I directly export |
symmetrize_operator for construction of custom operators
lkdvos
left a comment
There was a problem hiding this comment.
Except for removing the extra tests, looks great to me!
You might want to also immediately bump the version for a patch release?
symmetrize_operator for construction of custom operatorssymmetrize_operator for custom operator symmetrization
|
I still want to take a closer look at the t-J case with |
|
Let's not release 0.3.1 immediately. I also want to export the function to add charges in a separate PR. |
lkdvos
left a comment
There was a problem hiding this comment.
Apologies for the second round of review comments, feel free to ignore if you disagree, had some time to kill before a flight ;)
|
@lkdvos I skipped CI in the last commit since it affects documentation only. Can you help directly merge, or force start the tests? |
This PR exports module-specific
symmetrize_operatorfunctions to symmetrize custom operators.