Replace truncate! with truncate

[lkdvos]

This PR removes the use of truncate!, as it can be convenient for the pullback rules to have access to the truncated indices without having to repeat the logic of truncate.
Additionally the non-bang-method makes sense since our implementations do not alter the input arrays (and shouldn't for the pullbacks to remain valid).

Depends on #61

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines	Coverage Δ
ext/MatrixAlgebraKitChainRulesCoreExt.jl	83.73% <100.00%> (ø)
src/MatrixAlgebraKit.jl	100.00% <ø> (ø)
src/algorithms.jl	92.80% <ø> (ø)
src/implementations/eig.jl	99.02% <100.00%> (ø)
src/implementations/eigh.jl	94.48% <100.00%> (ø)
src/implementations/orthnull.jl	90.90% <100.00%> (ø)
src/implementations/svd.jl	92.28% <100.00%> (ø)
src/implementations/truncation.jl	93.05% <100.00%> (ø)

🚀 New features to boost your workflow:

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

[mtfishman]

@lkdvos I think all of our uses of truncate! don't actually mutate, so truncate makes more sense anyway.

When functions return multiple different things, I generally prefer to name them explicitly to indicate that, such as truncate_and_indices. That would leave open the possibility of defining truncate(args...) = first(truncate_and_indices(args...)) (analogous to how AD libraries have separate functions gradient(...) and value_and_gradient(...)), which I would find convenient since generally I have only needed the truncated factors, not the indices (or I call findtruncated directly to get the indices).

[lkdvos]

@mtfishman I understand what you mean, but given that this is an unexported function, mostly used for internal implementation purposes, I am not sure if this really matters? I think I prefer the shorter form, but obviously it doesn't actually make a difference so 🤷

[mtfishman]

@mtfishman I understand what you mean, but given that this is an unexported function, mostly used for internal implementation purposes, I am not sure if this really matters? I think I prefer the shorter form, but obviously it doesn't actually make a difference so 🤷

I kind of see your point, but also having a longer and more explicit name would make it clearer for downstream packages like BlockSparseArrays.jl what the function outputs when it gets overloaded and used, even if it is just for the developer. I'm not going to die on this hill, just sharing how I would design it if it was up to me. I suppose I'm a little confused by your comment though, should we not being using it in BlockSpareArrays.jl? I find it to be a useful abstraction layer.