Repository navigation
Conversation
| σs = abs.(collect(view(values, perm))) | ||
| start = firstindex(σs) | ||
| for k in eachindex(σs) | ||
| if k == lastindex(σs) || σs[k] - σs[k + 1] > max(strategy.atol, strategy.rtol * σs[k]) |
There was a problem hiding this comment.
This is probably not GPU friendly yet but I think that can be handled in a separate PR
There was a problem hiding this comment.
One thing I am wondering about for this functionality, is if there is ever a case where absolute tolerance makes sense. Like, suppose atol = 1e-6. Then by construction, all singular values smaller than mode == :up, if the truncation in strategy.strategy leads to a truncation at values smaller than atol, effectively there will be no truncation.
There was a problem hiding this comment.
Indeed I was a bit torn on this question as well, but for now decided to keep atol in case we think of a need for it later. It could make sense I suppose if you know a certain sector has small (like 1e-4) singular values which are never the less bounded from below?
| _ind_to_mask(values::AbstractVector, ::Colon) = trues(length(values)) | ||
| _ind_to_mask(values::AbstractVector, ind::AbstractVector{Bool}) = BitVector(ind) |
There was a problem hiding this comment.
There is a GPU-CPU related issue found by 🤖 that I don't understand...
There was a problem hiding this comment.
Yes indeed, this PR wasn't made with the GPU stuff in mind at all, I can handle that in a separate PR imo
There was a problem hiding this comment.
(I think BitArray just doesn't work on GPU lol)
| # walk the values in sorted order `perm`, and make sure each group of consecutive degenerate | ||
| # values is either kept or discarded as a whole cluster | ||
| function _keep_degenerate!(keep::AbstractVector{Bool}, values::AbstractVector, perm, strategy::TruncationDegenerate) | ||
| σs = abs.(collect(view(values, perm))) |
There was a problem hiding this comment.
Isn't the broadcasted abs. already doing a collect? This may yield a double collection/allocation/
There was a problem hiding this comment.
Hm, I'm not sure? If view(values, perm) lives on the GPU for example, would abs collect?
There was a problem hiding this comment.
Oh collect as in back to CPU. No than not. But I thought this PR was CPU only so far?
There was a problem hiding this comment.
It sort of accidentally supports the GPU by doing this collect which is a bit of a hack
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
Should address #101 and part of QuantumKitHub/TensorKit.jl#588
I added a keyword so users can pick whether to bias towards getting rid of the whole group of degenerate values or keeping them.