Optimization for fermion system

[emstoudenmire]

This PR modifies the code for compute_permfactor and parity_sign in src/fermions/fermions.jl. The resulting changes speed up DMRG calculations using the auto fermion system by about 3x.
The main changes are:

• Use Bumper to allocate the the oddp array in compute_permfactor. This was the main bottleneck.
• Pass the second argument to compute_permfactor as an array or tuple, instead of splatting it.
• Modify parity_sign to use boolean logic (less clear how much this helped but code is not more complicated afterward)

Note that this PR breaks the OpSum to TTN system in ITensorNetworks because of one line there which calls permfactor still using the "splatting" interface. I'll make a PR to ITensorNetworks updating that.

[mtfishman]

It is too bad that this technically breaks ITensorNetworks.jl, since that means we should mark it as a breaking release, otherwise users of ITensorNetworks.jl won't know what version of NDTensors.jl/ITensors.jl they should use to be compatible with ITensorNetworks.jl. To be specific about what can go wrong, once this is released, if we don't mark it as breaking, if a user updates their packages it could break their code using ITensorNetworks.jl through no fault of their own.

It is also unfortunate that ITensorNetworks.jl is using internals of NDTensors.jl/ITensors.jl since that makes these breakages more likely as we develop NDTensors.jl/ITensors.jl. For now, can we keep around the previous interface of compute_permfactor as a compatibility layer? A related question is, is the change to the interface needed? It doesn't look like the previous interface was problematic based on how I see it being used but maybe I'm missing something. Is the idea that sometimes the input is a Vector and sometimes it is a Tuple and splatting isn't friendly for Vector inputs?

If we do keep both interfaces, my preference would be that we keep the current interface of compute_permfactor intact, and then define a new function reduce_compute_permfactor or something like that to indicate one takes a splatted list of inputs and one takes a collection. One of them can call the other one, i.e. in that scenario compute_permfactor could call reduce_compute_permfactor.

I have to say I'm also a bit confused about the switch to using Bumper here, is that also related to the fact that the input may be a Vector or a Tuple and that's an approach that works better for Vector inputs? For Tuple inputs the current implementation makes sense to me.

Note that a strategy in ITensorNetworks.jl to avoid calling internals of NDTensors.jl/ITensors.jl, which may be a good strategy to take here, is code duplication, i.e. write a duplicate version of compute_permfactor in ITensorNetworks.jl and use that definition there instead of reaching into the internal NDTensors.jl/ITensors.jl implementation. That helps to protect us from changes to NDTensors.jl/ITensors.jl breaking ITensorNetworks.jl. In this case it is a bit too late for that since ITensorNetworks.jl already calls ITensors.compute_permfactor but we can do that going forward and/or switch over some of these kinds of cases to protect us later.

[mtfishman]

It looks like the ITensorMPS.jl downstream tests are failing to run properly, I think it is related to some recent changes to the CI infrastructure. I'll look into it.

[emstoudenmire]

Yes, good point about the breaking change. I believe there should be a way to make it non-breaking without losing any performance. Let me look into it.

The two performance related changes here are:

1. the use of Bumper is for performance reasons. I found that the current code, using MVector, is performing extremely slowly, much more slowly than just allocating a vector using Vector{Int}(undef,N). So the change to Bumper is to get rid of that slowdown, then I found that Bumper was performing somewhat better than allocating a vector, as it ideally should be. All of these changes showed up substantially in timings. (E.g. going from 28s for a DMRG sweep down to about 17s.)

2. changing the compute_permfactor to not have its input 'splatted' also made a measurable performance difference. That's also the change that turned out to be breaking, but I think we can get best of both here & I'll update the PR for that

[mtfishman]

Gotcha, I definitely believe you that the changes in this PR make the examples you are running faster, I just don't understand why.

[mtfishman]

I.e. for Tuple inputs the current code looks fine to me (though maybe I'm missing something) so the only explanation I could think of for why the new code is better is if Vectors are being input.

[emstoudenmire]

Ah, I see. Yes I think there are vector inputs sometime - I can check. Happy to discuss a bit more because I don’t fully understand why the previous code was slower.

[mtfishman]

Ok, I've fixed downstream testing of ITensorMPS.jl and added downstream testing of ITensorNetworks.jl, and it properly picked up that this PR breaks functionality in ITensorNetworks.jl.

[emstoudenmire]

Ok to close this, as it is superseded by #1712