Changes to the iterator interface

[jack-dunham]

This PR attempts to define an interface for the iteration pattern that is similar to that of DifferentialEquations (as described in the Google doc), however in this case it was not as simple. I have described why and what I did below:

AbstractNetworkIterator

The iterate function in Julia really describes how one should transition between states, i.e. from A to B.
For example, when iterating through SweepIterator, one must define the new RegionIterator for the next sweep. This is something that is suitable to be wrapped in iterate as it essentially is the step required to transition from one sweep to the next. What is less obvious is when to perform the region iteration itself such that the code

for _ in sweep_iterator end

performs the actual calculation. The region iteration is not really something that is part of "transitioning from sweep A to B" but rather work that is performed during a given sweep. This makes it somewhat difficult to reconcile the state of the data (such as the tensors themselves etc) with the state of the iterator.
Considering what iteration lowers to:

next = iterate(iter)
while next !== nothing
    (item, state) = next
    # body (this is where any call to a callback function would occur)
    next = iterate(iter, state)
end

One has computed and iterated before any callback function. Each callback essentially acts before each computation, which is perhaps little awkward, but not necessarily a problem. What is a problem is that the first step is an exception to this; we don't get to execute a callback at step 1 before any computation happens.

To avoid this problem, and allow the callback to execute after the computation I have decide to separate out the "transition" step and the computation step into two functions increment! and compute! respectively. This also makes it clear which code is responsible for moving the iterator from A to B, and which code is responsible for performing computation while in state A (or B etc). We now also make the first call to increment! implicit. That is, the iterator should, when initialized, run the necessary code such that it is ready for computation at the first step. In a way, this is as if we have transition from some abstract 0th state to the 1st state. In the first call to iterate only the call to increment! is then skipped.

This is the specific interface of AbstractNetworkIterator. An AbstractNetworkIterator is a stateful iterator and can be summarized by the following iterate definition:

function Base.iterate(NI::AbstractNetworkIterator, init=true)
  done(NI) && return nothing
  init || increment!(NI)
  rv = compute!(NI)
  return rv, false
end

The function done just checks that the iteration is complete. Note, we increment before we compute, therefore we require

done(NI::AbstractNetworkIterator) = state(NI) >= length(NI)

with a >= rather than > to avoid over shooting by 1. Simple subtypes of AbstractNetworkIterator can use the above method by defining state and Base.length, however one can instead choose to dispatch on done itself for more complicated cases (see SweepIterator for an example).

The code

for _ in iter
    callback(iter)
end

when iter::AbstractNetworkIterator now executes the callback function after the computation step, but before the work has been done to transition to the next state.

More details

• The compute! function be default just returns the iterator itself (and does nothing else). One can choose to return something else to be used in the body of a loop if desired.
• The function increment! has no default implementation on purpose
• See the PauseAfterIncrement adapter iterator as one example of how the interface can be used. This is a general version of the proposed adapter that would allow manual iteration over the region.

Other changes

Misc

• SweepIterator now has a single type parameter Problem that can be used for dispatch (useful for default callbacks, see below).
• I've changed some function calls to be explicit calls to constructors to make it clearer when one is constructing rather than getting.
• RegionIterator now has an additional field, sweep that is constant and simply stores the current sweep the region iteration is part of. This is to avoid constantly passing around this sweep keyword when dispensing the sweep iterator in favor of the simpler region iterator.

sweep_solve

• The function sweep_solve is now just a wrapper over the iterators that allows callbacks to be passed (and therefore requires PauseAfterIncrement).
• The callback functions now take the entire sweep_iterator as the only positional argument.
• I've allowed passing in arbitrary kwargs for now.
• The sweep_callback and region_callback functions have has default_ prepended to their function names to distinguish them from the keyword argument (and to make their purpose clear).

Notes

• I tested the relevant tests in solvers subdirectories but I couldn't get the full test suite to run for some reason. Might be something up with my environment.
• I will add some unit tests!

[jack-dunham]

• Reconsider the done function. The name is misleading as the computation is not "done". The purpose of this function is really to check if the iterator is in the final state.
• Rename PauseAfterIncrement to something more sensible, i.e. NoCompute or similar.
• Add an adapter that spits out the region iterator EachRegion, e.g.
• Consider if it makes sense to embrace mutability during the region_step function. Perhaps this then informs the interface for extract insert and update.
• Consider rolling which_sweep field of SweepIterator into RegionIterator.

[mtfishman]

Looks like there is an issue with product(::TTN, ::TTN) not being defined.

[jack-dunham]

I think that's fair enough. The sketch I had was that you would choose to either the method on the type if you didn't need any runtime info to generate the defaults, or the values in the event that you did. Ideally you wouldn't have both methods.
The reason for this is so you can return the defaults without having to construct instances of types if it is not necessary to do so.

I don't think that use case is relevant to anywhere in the code yet so I agree; a bridge we can cross when it becomes necessary.

I can change to @define_default_kwargs which is a bit more specific about what it's doing (defining a method for default_kwargs).

[jack-dunham]

Might as well include the @with_defaults macro and sweep_solve method for EachRegion in this PR in the meantime.

[emstoudenmire]

This PR has a green light from me, after remaining review comments are resolved or answered.

[mtfishman]

@jack-dunham Miles recommended just skipping the broken TTN fitting tests, it requires some more work to get those working.

[jack-dunham]

The tests in the following files are broken in the branch of ITensorNetworks.jl that this PR modifies, i.e. the original network solvers code:

test_ttn_contract.jl
test_ttn_dmrg.jl
test_ttn_dmrg_x.jl
test_ttn_linsolve.jl
test_ttn_tdvp.jl
test_ttn_tdvp_time_dependent.jl

Considering this is a lot of tests, it might be worth investigating if most can be salvaged and ported into the new interface. I think the code that these tests were based on is in src/solvers/previous_interfaces/.

Perhaps it's best to merge this with those tests skipped and then deal with in the other PR. This PR doesn't break any further tests. i.e. with those tests skipped, all remaining tests pass.

TODO:

• Excise default_kwargs from this PR.
• Rename sweep_solve to sweep_solve! in line with Julia conventions
• For consistency, ensure all mutating functions are returning the mutated argument (and any other relevant data)
• The EachRegion adapter should return the iterator from compute! for consistency.

[mtfishman]

I think it makes sense to skip broken tests if they were already broken in Miles's PR. However, I would prefer that we left the file names as they were, and instead wrap the tests in @test_skip:

@test_skip begin
  @testset ...
  end
end

so it is recorded in the test results.

Also, I'm a bit confused that all of those tests were broken, @emstoudenmire I thought you were using TTN DMRG and TDVP?