Rework fixed-point differentiation#386
Conversation
lkdvos
left a comment
There was a problem hiding this comment.
Left some very small comments, but this looks pretty good to me actually
|
After the build completes, the updated documentation will be available here |
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
pbrehmer
left a comment
There was a problem hiding this comment.
This is really great, thanks! I find that it definitely cleans up and improves the entire (fixed-point) gradient framework. I only left a few minor comments.
|
|
||
| boundary_alg = (; tol = 1.0e-8, alg = :SimultaneousCTMRG, trunc = (; alg = :FixedSpaceTruncation)) | ||
| gradient_alg = (; tol = 1.0e-6, maxiter = 10, alg = :LinSolver) | ||
| gradient_alg = (; tol = 1.0e-6, maxiter = 10, solver_alg = (alg = :GMRES)) |
There was a problem hiding this comment.
With all the example changes we have to make sure to rerun all of them such that the rendered version reflect the changes.
| @@ -1,12 +1,15 @@ | |||
| abstract type GradMode{F} end | |||
| abstract type GradMode{A} end | |||
There was a problem hiding this comment.
Although this is not really necessary, what do we think about renaming this to GradientMode{A}? I think the abbreviation is not really useful here and GradientMode sounds better in my opinion. (And we're already renaming and breaking a lot anyway.)
There was a problem hiding this comment.
I'm happy to rename it. What do you think about GradientAlgorithm as an alternative option? I was never too sure about Mode to be honest.
There was a problem hiding this comment.
I like GradientAlgorithm even better since it is more consistent with the rest of our algorithm struct names.
There was a problem hiding this comment.
Then I would go for that.
Attempt at reorganizing the fixed-point differentiation approach. The main motivation was to remove the
iterschemekeyword that was previously used to select a specific flavor of fixed-point differentiation, which has now become trivial since we only support one value.As a result it now made more sense to switch the logic around: instead of using a solver algorithm struct as a representation of a gradient algorithm where the type parameter specifies how the gradient is actually computed, I now represent the gradient algorithm as a specific type and store the solver algorithm inside that. This involved coming up with a new name, which ended up being
FixedPointGradient, and wiring this into the algorithm selection. It looks a bit silly now since there's only one option, but hopefully this is a good setup for adding different styles of implicit differentiation.Very open to suggestions and changes, let me know what you think.