Reorganize 'ortho! X vs Y' loop - #11
Conversation
Hm. Actually this check should really be in ortho!(X) (so we're able to orthogonalize even a matrix of zeros)... The same for the column normalizations probably. Would that be terribly expensive? If so we can use norminvR as a proxy for this (in the rare case where norminvR is much too big, then we try to drop columns or something) (also while you're at it: return (; X=U*V', nchol=100, growth_factor=1)) this should really be Inf(T) for growth_factor
the original point is that norm(BY'X) is costly but norm(BYX) (with already computed BYX) is not. I would expect this change to be a potential negative on CPU. Not completely against getting rid of it though if the impact is minimal and it really improves things on a GPU
yup this is nice! |
I've abandoned the idea of trying to pass optimizations such as in PR #8, but I learned a few things in the process. It is possible to reorganize the
ortho! X vs Yloop a little to gain performance, without any algorithmic change. This benefits both CPU and GPU.The changes:
Perform the
drop_small!test at the end of the iterative process, and restart completely if needed. This is an extremely rare occurrence, and I have never seen it happen in a real calculation. Since it is a rather expensive test, we gain a lot by only doing it once per function call. The overall diagonalization becomes twice as long if a problem is spotted, but that essentially never happens.Get rid of the
norm(BYX)test. As already suggested in the comments, this is an expensive test. Also, actually performing that test goes against the said comments completely. From my checks, essentially all loops are escaped via theestimated_errorcheck anywaysMove to in-place
cholesky!: we never need the Gramm matrix anyways, so we might as well avoid some allocations. Effect is quite anectodical, but it does not hurt.Performance-wise, I observe this:

We get a 10-15% speedup overall, which really comes for free this time. I also added the AI agent's proposition to fuse the Cholesky kernels for reference. This shows extra potential, since the optimizations are orthogonal to this PR. It is quite a hard effort to clean-up the AI code though, and I failed to do so in a previous attempt.