Skip to content

Conversation

@Yue-Zhengyuan
Copy link
Member

@Yue-Zhengyuan Yue-Zhengyuan commented Oct 2, 2025

This PR greatly simplifies the TJOperators module by creating the t-J operators from projecting the corresponding Hubbard operators, as discussed in #20. The tests on t-J model are unmodified.

I finally decide to keep the slave_fermion option for convenience... But this requires me to copy some utility function from MPSKit and PEPSKit in order to add the fermion-Z2 charge.

TODO:

  • Since the t-J operator functions are now generated with @eval, the docstrings are missing and need to be added back.

@codecov
Copy link

codecov bot commented Oct 2, 2025

Codecov Report

❌ Patch coverage is 92.06349% with 5 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/tjoperators.jl 91.66% 4 Missing ⚠️
src/hubbardoperators.jl 66.66% 1 Missing ⚠️
Files with missing lines Coverage Δ
src/TensorKitTensors.jl 100.00% <ø> (ø)
src/utils.jl 100.00% <100.00%> (ø)
src/hubbardoperators.jl 96.83% <66.66%> (+0.64%) ⬆️
src/tjoperators.jl 92.00% <91.66%> (-6.98%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Yue-Zhengyuan Yue-Zhengyuan requested a review from lkdvos October 2, 2025 02:37
@lkdvos
Copy link
Member

lkdvos commented Oct 18, 2025

Apologies for taking some time to get to this, I have to admit that it slightly slipped my attention. Do you mind if I add some changes here?

Some general comments:

While I agree that having to maintain less code is nice, I'm less happy with the definitions of tensorexpr and the generated functions. This might just be me, but it feels a lot less "light-weight" than I had envisioned for this package.
I also would want to slightly change the implementation to avoid floating points, as the contractions will lose a little bit of precision. I think @Jutho already mentioned this but it would be better if we could define the projectors with integers, and only using *.
I'll have a look in a bit to see if I can think of anything

[EDIT] ignore most of what I just said, I see you already did this, I'm apparently not very awake

@Yue-Zhengyuan
Copy link
Member Author

Yue-Zhengyuan commented Oct 18, 2025

tensorexpr, twistdual etc (the entire utils.jl file) can be removed if the slave_fermion keyword is ditched. I'm OK if you decide to do this. Though I'll have to manually redefine the t-J functions in my personal code.

@lkdvos
Copy link
Member

lkdvos commented Oct 18, 2025

I've updated the approach a bit, in an attempt to simplify.

The biggest change is that I've removed the slave_fermion::Bool keyword from the general method definitions, which I think simplifies things a bit, but then added back a method for performing the basis transformation.
In general, I agree that _fuse_ids might be useful, so I've added the fuse_local_operators function to have a generic approach to this, which we can additionally consider exporting and using it or adding it to e.g. bosonic operators with a non-trivial charge, etc.

Keep in mind that this is just a suggestion, so feel free to give your opinions on the changes I made, we can definitely revert/alter some things if you disagree!

Comment on lines 94 to 98
@eval begin
hub_doc = (@doc HubbardOperators.$opname).text[1]
tJ_doc = replace(hub_doc, "[spin_symmetry::Type{<:Sector}])" => "[spin_symmetry::Type{<:Sector}]; slave_fermion::Bool = false)") * "Use `slave_fermion = true` to switch to the slave-fermion basis.\n"
@doc (tJ_doc) $opname
end
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to restore the slave_fermion keyword, which need me to replace and add some text to the corresponding Hubbard docstring.

@lkdvos
Copy link
Member

lkdvos commented Oct 19, 2025

Hahah I was hoping to avoid the keyword argument since I don't like it too much, and it does complicate the docstrings etc a bit, but I guess this also works. I'll try and go over it tomorrow so we can finalize

Comment on lines 166 to 168
| |0⟩ | h⁺|0⟩ |
| u⁺|0⟩ | bꜛ⁺|0⟩ |
| d⁺|0⟩ | bꜜ⁺|0⟩ |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's a great idea to call both vacua |0⟩ here. I added the prime to distinguish them (unless I'm mistaken they really are different?) because from this table, it looks like h⁺ = I etc. If you have a better notation for the different vacuum that obviously also ok, but I wouldn't use the same notation for both.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My point is that in the slave-fermion representation, the basis state with FermionParity(1) is indeed h⁺|0⟩, where |0⟩ is the true vacuum (the FermionParity(0) state in the usual representation). So it shouldn't be written as h⁺|0'⟩, suggesting |0'⟩ is different from |0⟩.

Similarly, the usual u⁺|0⟩ is mapped to u⁺h⁺|0⟩ = (h bꜛ⁺)h⁺ |0⟩ = bꜛ⁺ |0⟩ in the slave fermion basis.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might really just be confused though, apologies since this is not necessarily a model I usually interact with.
Did I manage to just confuse myself by putting it in a table and somehow trying to relate the different states in the same row? I like the visual of the table, but maybe this was just not necessarily the right approach

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

| tJ basis | slave-fermion |
| -------- | ------------- |
|   |0⟩    |      h⁺|0⟩    |
|  u⁺|0⟩   |     bꜛ⁺|0⟩    |
|  d⁺|0⟩   |     bꜜ⁺|0⟩    |

The states in the same row are indeed related. Just interpret it as "states in the left column are mapped to the right column by the basis transformation".

lkdvos
lkdvos previously approved these changes Oct 20, 2025
lkdvos
lkdvos previously approved these changes Oct 20, 2025
@Yue-Zhengyuan
Copy link
Member Author

Can't merge because of low patch coverage. But the missing lines are mostly about raising errors and operator functions with omitted arguments.

@lkdvos lkdvos merged commit fad7491 into main Oct 21, 2025
8 of 9 checks passed
@lkdvos lkdvos deleted the zy-tJ-overhaul branch October 21, 2025 01:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants