-
Notifications
You must be signed in to change notification settings - Fork 21
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
Classical intensities as density #272
Conversation
5a05f7c
to
4617a99
Compare
71a30af
to
f624b94
Compare
examples/04_GSD_FeI2.jl
Outdated
) | ||
|
||
heatmap!(ax_bottom,1:size(is_binned,1), ωs, is_binned; | ||
colorrange=(0.0,0.05), | ||
colorrange=(0.0, 1.5e-6), |
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.
Did we accidentally introduce a Δω factor into binned intensities? These are already integrated quantities, so no Δω should appear, and the colorrange should not need to change.
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.
This was the figure that was already "broken" (i.e., appeared blank) before anything was added by this PR. In other words, this change was essentially unrelated to the rest of the PR.
@Lazersmoke I don't think any of the binning functions call intensities_interpolated
, so they should not be affected by this PR. All the binning tests still pass. But the binned results in the FeI2 tutorial appear appear like they already needed some plotting tweaks before this PR. See binned plot here. The change here just tweaks the plotting limits so that the results show up, as seen here.
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.
Looks good to me, pending Sam's comments on binned intensities. Thanks David. |
Hi David, thanks for writing this up. I think the better way to make this change is to remove the 1/sqrt(n all omega) from the time fft normalization factor. The complete end to end calculation in the time direction is then:
|
(per the discussion around #246 , the overall color scale should have not changed during that, but it turns out it actually did because the aforementioned inclusion of the factor in step 3 happened when that was merged) |
See #273 |
This PR
intensities_interpolated
bysc.Δω
so that the returned values are densities. Ifsc.Δω=NaN
(becausesc
is an instant structure factor) divides by 1 instead.colorrange
s of heatmap plots in examples.