Implement keyword selector

[pbrehmer]

In this PR we implement a keyword selector function which parses optimization kwargs onto the corresponding algorithm structs and outputs a PEPSOptimize to fixedpoint. This simplifies the user interface since one doesn't have to create multiple nested algorithm structs.

The same could be done also for leading_boundary since it is another frequently-used function with many algorithmic parameters.

This idea follows KrylovKit's design which supplies a user-friendly kwarg-based method and an "expert" method. See e.g. the eigselector.

[codecov]

Codecov Report

Attention: Patch coverage is 82.20859% with 29 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/algorithms/select_algorithm.jl	85.29%	15 Missing ⚠️
src/algorithms/optimization/peps_optimization.jl	58.82%	7 Missing ⚠️
src/algorithms/ctmrg/projectors.jl	33.33%	4 Missing ⚠️
src/Defaults.jl	85.71%	1 Missing ⚠️
src/algorithms/ctmrg/ctmrg.jl	90.90%	1 Missing ⚠️
src/utility/svd.jl	80.00%	1 Missing ⚠️

Files with missing lines	Coverage Δ
src/PEPSKit.jl	100.00% <ø> (+12.50%)	⬆️
src/algorithms/ctmrg/sequential.jl	98.36% <100.00%> (ø)
src/algorithms/ctmrg/simultaneous.jl	98.18% <100.00%> (ø)
...rithms/optimization/fixed_point_differentiation.jl	93.61% <100.00%> (-1.29%)	⬇️
src/algorithms/time_evolution/simpleupdate.jl	87.01% <ø> (ø)
src/operators/transfermatrix.jl	64.86% <ø> (ø)
src/Defaults.jl	85.71% <85.71%> (ø)
src/algorithms/ctmrg/ctmrg.jl	89.65% <90.90%> (+0.56%)	⬆️
src/utility/svd.jl	85.13% <80.00%> (-0.79%)	⬇️
src/algorithms/ctmrg/projectors.jl	86.95% <33.33%> (-8.50%)	⬇️
... and 2 more

🚀 New features to boost your workflow:

• ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[pbrehmer]

I will finish this up tomorrow: I want to further test around and I need to finish up the docstrings. I will then also make the tests use the kwarg-based interface since that should get rid of a few lines. Perhaps we can get this merged tomorrow :-)

[lkdvos]

I sadly won't have time to really look over this in detail today (and Monday is a holiday here), but I will look into it next week. Do you mind if I try out some changes and push them to your branch if I get to it?

[pbrehmer]

In the end it turned out that supporting Symbols, Type{Algorithm} and Algorithm does require a bit of care. Most of the complexity is now contained in the select_algorithm methods which also the algorithm struct constructors now call. I tried to update the docstrings to list all the options by also stating the possible Symbols.

From the user point of view I still feel that setting parameters via nested NamedTuples is a bit complicated but there is probably no way which would make this substantially less complicated - the PEPS stack is just quite complicated.

[lkdvos]

Thanks a lot for all of the work you've put into this already, it's really coming together nicely and I like the improvements a lot!

I've made some small changes still, and have some general comments for which I apologize in advance, since some of these might contradict things I have hinted at earlier.

I really like the select_algorithm pattern for functions, I think that makes a lot of sense and cleanly separates out handling the kwargs from the actual algorithm itself.
I don't like this pattern as much for the select_algorithm(T::Type) versions, because there we're not selecting an algorithm for implementing T(args...), we really are just constructing a alg::T. I think it makes a bit more sense to keep these as actual constructors. For example, we can also define constructors for abstract supertypes that return a concrete subtype and selects that based on the provided (keyword) arguments.
If you agree with this, I should be able to make some time for implementing this this afternoon, if you are getting sick of working on this for a bit I can definitely help out ;)

I'm a bit torn between having the select_algorithm in a separate file for all functions, or having them bundled with their respective implementations (ie closer to where the docstring would be). I think I like both, but I'm slightly worried about the order of having to define things, ie that some types/functions might not exist yet etc. Perhaps that's not really a problem and we can always define select_algorithm after everything is already defined though. Just wanted to explicitly bring this up and see what we all think.

I've made a slight formatting change where I added a space before the : that comes after the name of the kwarg, because I felt like there are too many :: and : with the symbols etc and wanted to cleanly separate out the code part from the text part a bit more. Let me know if you don't like it and I'll change it back.

Are we now still using the (; alg=::Type{}) form anywhere? I think I may have suggested this at some point, but given that there are now symbols everywhere, I think it would be fine (preferential even) to just drop that syntax altogether. It seems a bit artificial, and does not really give any benefit over the symbols I think.
I've added some dictionaries instead of the if statements for these symbols, mostly to allow them to be externally extensible if anyone would want that.

Again, if you want I can make some time today for working some of these things in, feel free to ask for help :)

[lkdvos]

This pattern I like a bit less: we already have a constructor, and dispatching that to select_algorithm seems like a reroute that does not bring to much to the table. In particular, calling HalfInfiniteProjector on something really has to give back a ::HalfInfiniteProjector, so pushing it to a select_algorithm(::Type{ProjectorAlgorithm}) feels a bit wrong.

[pbrehmer]

I don't like this pattern as much for the select_algorithm(T::Type) versions, because there we're not selecting an algorithm for implementing T(args...), we really are just constructing a alg::T. I think it makes a bit more sense to keep these as actual constructors.

Actually I'm also somewhat conflicted on this. I think I went this route since the select_algorithm functions compose a bit and I didn't see (at the time) how to easily do this with the algorithm constructors without duplicating a lot of code. Please feel free to change this and implement the corresponding constructors if you feel it makes things a bit more consistent and cleaner! (I am a bit sick of working on this I have to admit ;))

I'm a bit torn between having the select_algorithm in a separate file for all functions, or having them bundled with their respective implementations

I'm also torn on this and I mostly did that to have all selections in one place since they do depend on each other. I think I'd be fine with both ways but I previously did have issues with things not yet being defined which is why I put the select_algorithm.jl file at the end of the includes.

Are we now still using the (; alg=::Type{}) form anywhere? I think I may have suggested this at some point, but given that there are now symbols everywhere, I think it would be fine (preferential even) to just drop that syntax altogether.

No we don't use that form anymore really. Somehow I thought that we wanted to keep this but in hindsight I don't know why I didn't remove this right away. It just makes things more complicated and blows up the docstrings. I would definitely be in favor of removing this.

[pbrehmer]

I will remove the (; alg::Type{}=alg) syntax now and adapt the docstrings but then feel free to bring the PR over the finish line :-) Of course I'm happy to help!

[lkdvos]

I'll try and go through this tomorrow to see if there are any remaining things, but should be more or less finished, so we can merge this Monday