-
Notifications
You must be signed in to change notification settings - Fork 34
Fix thermal_dm and enr_thermal_dm for extreme cases
#614
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 6 commits
143a63c
9f75e3e
f9287ba
36fb4d9
2b3b33b
c1fb579
0d91305
1eb3b38
3a26bab
9d99106
7ac713d
db7da3a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -118,13 +118,14 @@ Density matrix for a thermal state (generating thermal state probabilities) with | |
| !!! warning "Beware of type-stability!" | ||
| If you want to keep type stability, it is recommended to use `thermal_dm(N, n, sparse=Val(sparse))` instead of `thermal_dm(N, n, sparse=sparse)`. See [this link](https://docs.julialang.org/en/v1/manual/performance-tips/#man-performance-value-type) and the [related Section](@ref doc:Type-Stability) about type stability for more details. | ||
| """ | ||
| function thermal_dm(N::Int, n::Real; sparse::Union{Bool,Val} = Val(false)) | ||
| β = log(1.0 / n + 1.0) | ||
| data = [exp(-β * ComplexF64(j)) for j in 0:(N-1)] | ||
| function thermal_dm(N::Int, n::T; sparse::Union{Bool,Val} = Val(false)) where {T<:Real} | ||
| exp_minus_β = 1 - (1 / (n + 1)) # use "1-1/(n+1)" instead of "n/(n+1)" for numerical stability (works for n=0 and Inf) | ||
| P_n = _complex_float_type(T)[exp_minus_β^j for j in 0:(N-1)] | ||
| P_n /= sum(P_n) | ||
|
||
| if getVal(sparse) | ||
| return QuantumObject(spdiagm(0 => data ./ sum(data)), Operator(), N) | ||
| return QuantumObject(spdiagm(0 => P_n), Operator(), N) | ||
| else | ||
| return QuantumObject(diagm(0 => data ./ sum(data)), Operator(), N) | ||
| return QuantumObject(diagm(0 => P_n), Operator(), N) | ||
| end | ||
| end | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately this is way slower than the exp version.
thermal_dm(3000, 0.2; sparse=Val(true))becomes 50% slower than before.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But this implementation gives correct results for all known extreme cases. I think this is more important.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And that's why I was talking about branching on the finiteness of beta. Since precomputing the normalization factor is problematic for dealing with
nbar=Inf, simply replacing the first element in the original array already works and is faster, i.e.This is doing basically the same thing as the slower version since the
powfunction fallbacks to log and exp.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I didn't fully understand your point in the beginning
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the other PR @yuyichao what saying that perhaps it is better to preallocate the vector and then filling, to avoid comprehension. Is it still true here?
I also would avoid the use of the
Boltzmannfunction we have just introduced.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The avoiding comprehension part was mainly to not write too much code in the comprehension (for coding style only). It is true that comprehension can basically only be as performant as loop and not better but that's not too important.
The code in the loop (and therefore would've been in comprehension) was more complex in the other PR since I wanted to reuse the result for the summation computation. However, since the analytical expression to calculate the sum had issue with infinite nbar and is therefore disgarded, that's less of a problem (granted
1 + 1/nis already computed but that has always been the case) and the code in the loop is fairly simple for the current version.