More reliable binning + unit tests

src/Intensities/Binning.jl

@@ -39,7 +39,10 @@ function Base.show(io::IO, ::MIME"text/plain", params::BinningParameters)
         else
             printstyled(io, @sprintf("⊡ %5d bins",nbin[k]); bold=true)
         end
-        @printf(io," from %+.3f to %+.3f along [", params.binstart[k], params.binend[k])
+        bin_edges = axes_binedges(params)
+        first_edges = map(x -> x[1],bin_edges)
+        last_edges = map(x -> x[end],bin_edges)
+        @printf(io," from %+.3f to %+.3f along [", first_edges[k], last_edges[k])
         axes_names = ["x","y","z","E"]
         inMiddle = false
         for j = 1:4
@@ -63,12 +66,8 @@ Base.getproperty(params::BinningParameters, sym::Symbol) = sym == :numbins ? cou
 
 function Base.setproperty!(params::BinningParameters, sym::Symbol, numbins)
     if sym == :numbins
-        binwidth = (params.binend .- params.binstart) ./ numbins
-
         # *Ensure* that the last bin contains params.binend
-        binwidth .+= eps.(binwidth) 
-
-        setfield!(params,:binwidth,binwidth)
+        params.binwidth .= (params.binend .- params.binstart) ./ (numbins .- 0.5)
     else
         setfield!(params,sym,numbins)
     end
@@ -90,9 +89,9 @@ function BinningParameters(binstart,binend,binwidth;covectors = [1 0 0 0; 0 1 0
 end
 
 function BinningParameters(binstart,binend;numbins,kwargs...)
-    binwidth = (binend .- binstart) ./ numbins
-    binwidth .+= eps.(binwidth)
-    return BinningParameters(binstart,binend,binwidth;kwargs...)
+    params = BinningParameters(binstart,binend,[0.,0,0,0];kwargs...)
+    params.numbins = numbins # Use the setproperty to do it correctly
+    params
 end
 
 """
@@ -115,7 +114,10 @@ end
 # Find an axis-aligned bounding box containing the histogram
 function binning_parameters_aabb(params)
     (; binstart, binend, covectors) = params
-    bin_edges = [binstart binend]
+    bin_edges = axes_binedges(params)
+    first_edges = map(x -> x[1],bin_edges)
+    last_edges = map(x -> x[end],bin_edges)
+    bin_edges = [first_edges last_edges]
     this_corner = MVector{4,Float64}(undef)
     q_corners = MMatrix{4,16,Float64}(undef)
     for j = 1:16 # The sixteen corners of a 4-cube
@@ -206,18 +208,17 @@ function unit_resolution_binning_parameters(ωvals,latsize,args...;units = :abso
     total_size = max_val .- min_val
 
     binwidth = total_size ./ (numbins .- 1)
-    binwidth = binwidth .+ eps.(binwidth)
     binstart = (0.,0.,0.,minimum(ωvals)) .- (binwidth ./ 2)
-    binend = (maxQ[1],maxQ[2],maxQ[3],maximum(ωvals)) .+ (binwidth ./ 2)
+    binend = (maxQ[1],maxQ[2],maxQ[3],maximum(ωvals)) # bin end is well inside of last bin
 
     params = BinningParameters(binstart,binend,binwidth)
 
     # Special case for when there is only one bin in a direction
     for i = 1:4
         if numbins[i] == 1
-            params.binstart[i] = min_val[i]
-            params.binend[i] = min_val[i] + 1.
             params.binwidth[i] = 1.
+            params.binstart[i] = min_val[i] - (params.binwidth[i] ./ 2)
+            params.binend[i] = min_val[i]
         end
     end
 
@@ -232,11 +233,17 @@ end
 
 unit_resolution_binning_parameters(sf::StructureFactor; kwargs...) = unit_resolution_binning_parameters(ωs(sf),sf.latsize,sf;kwargs...)
 
-function unit_resolution_binning_parameters(ωvals::Vector{Float64})
+function unit_resolution_binning_parameters(ωvals::AbstractVector{Float64})
+    if !all(abs.(diff(diff(ωvals))) .< 1e-12)
+      @warn "Non-uniform bins will be re-spaced into uniform bins"
+    end
+    if length(ωvals) == 1
+      error("Can not infer bin width given only one bin center")
+    end
     ωbinwidth = (maximum(ωvals) - minimum(ωvals)) / (length(ωvals) - 1)
-    ωbinwidth += eps(ωbinwidth)
     ωstart = minimum(ωvals) - ωbinwidth / 2
-    ωend = maximum(ωvals) + ωbinwidth / 2
+    ωend = maximum(ωvals)
+
     return ωstart, ωend, ωbinwidth
 end
 
@@ -282,10 +289,10 @@ function slice_2D_binning_parameters(ωvals::Vector{Float64},cut_from_q,cut_to_q
     cotransverse_center = cotransverse_covector ⋅ cut_from_q
 
     ωstart, ωend, ωbinwidth = unit_resolution_binning_parameters(ωvals)
+    xstart, xend, xbinwidth = unit_resolution_binning_parameters(range(start_x,end_x,length = cut_bins))
 
-
-    binstart = [start_x,transverse_center - cut_width/2,cotransverse_center - cut_height/2,ωstart]
-    binend = [end_x,transverse_center + cut_width/2,cotransverse_center + cut_height/2,ωend]
+    binstart = [xstart,transverse_center - cut_width/2,cotransverse_center - cut_height/2,ωstart]
+    binend = [xend,transverse_center,cotransverse_center,ωend]
     numbins = [cut_bins,1,1,length(ωvals)]
     covectors = [cut_covector... 0; transverse_covector... 0; cotransverse_covector... 0; 0 0 0 1]
 
@@ -314,7 +321,7 @@ The following alternative syntax can be used to compute bin centers for a single
     axes_bincenters(binstart,binend,binwidth)
 """
 function axes_bincenters(binstart,binend,binwidth)
-    bincenters = []
+    bincenters = Vector{AbstractRange{Float64}}(undef,0)
     for k = 1:length(binstart)
         first_center = binstart[k] .+ binwidth[k] ./ 2
         nbin = count_bins(binstart[k],binend[k],binwidth[k])

test/test_binning.jl

@@ -0,0 +1,76 @@
+@testitem "Binning" begin
+
+    # Test constructor
+    lo = [0.,0.,0.,0.]
+    hi = [1.,1.,1.,1.]
+    nbins = [1,2,3,4]
+    params = BinningParameters(lo,hi;numbins = nbins)
+    @test params.numbins == nbins
+    @test all(isfinite.(params.binwidth))
+
+    # Ensure it works for the edge case of integrated bins
+    params = BinningParameters(lo,hi;numbins = [1,1,1,1])
+    @test all(isfinite.(params.binwidth))
+
+    @test_warn "Non-uniform" unit_resolution_binning_parameters([0.,1,3,7])
+
+    sys = System(Sunny.diamond_crystal(),(4,1,1),[SpinInfo(1,S=1/2)],:SUN,seed=1)
+    randomize_spins!(sys)
+    sf = DynamicStructureFactor(sys;Δt = 1.,nω=3,ωmax = 1.)
+    @test_nowarn unit_resolution_binning_parameters(sf)
+    params = unit_resolution_binning_parameters(sf)
+    @test params.numbins == [4,1,1,3]
+
+    # Ensure insensitivity to small changes in bin size
+    params.binwidth[2] = 1.
+    params.binwidth[3] = 1. - eps(1.)
+    @test params.numbins[2] == params.numbins[3]
+
+    # Test that parameters can be reconstructed from bin centers
+    bcs = axes_bincenters(params)
+    @test params.numbins == map(x -> length(x),bcs)
+    @test_throws "Can not infer bin width" unit_resolution_binning_parameters(bcs[2])
+    params.numbins = [8,2,2,6] # Give it more bins so it *can* infer the width
+    bcs = axes_bincenters(params)
+    bps = map(unit_resolution_binning_parameters,bcs)
+    new_params = BinningParameters(map(x -> x[1],bps),map(x -> x[2],bps),map(x -> x[3],bps))
+    @test all(isapprox.(new_params.binstart,params.binstart;atol=1e-12))
+    @test all(isapprox.(new_params.binend,params.binend;atol=1e-12))
+    @test all(isapprox.(new_params.binwidth,params.binwidth;atol=1e-12))
+    @test new_params.numbins == params.numbins
+
+    # SQTODO:
+    # Test that parameters can be reconstructed from bin edges
+    #bes = Sunny.axes_binedges(params)
+    #bps = map(x -> unit_resolution_binning_parameters,bes)
+
+    # Ensure insensitivity to small changes in bin size
+    # after setting the bin number
+    params.numbins = [1,1,7,1]
+    bins_before = params.numbins[3]
+    params.binwidth[3] = params.binwidth[3] - eps(params.binwidth[3]) # Minus
+    @test bins_before == params.numbins[3]
+    params.numbins = [1,1,7,1]
+    bins_before = params.numbins[3]
+    params.binwidth[3] = params.binwidth[3] + eps(params.binwidth[3]) # Plus
+    @test bins_before == params.numbins[3]
+
+    params = unit_resolution_binning_parameters(sf)
+
+    # TODO: Test broadening
+    is, counts = intensities_binned(sf, params)
+
+    is_golden = [2.452071781061995; 0.8649599530836397; 1.1585615432377976; 0.2999470844988036;;;; 0; 0; 0; 0;;;; 0; 0; 0; 0]
+    @test isapprox(is,is_golden;atol = 1e-12)
+    @test all(counts .== 1.)
+
+    is, counts = powder_average_binned(sf, (0,6π,6π/4))
+
+    is_golden = [4.475593277383433 0 0; 17.95271052224501 0 0; 51.13888001854976 0 0; 45.72331040682036 0 0]
+    counts_golden = [3.0 3.0 3.0; 15.0 15.0 15.0; 28.0 28.0 28.0; 39.0 39.0 39.0]
+    @test isapprox(is,is_golden;atol = 1e-12)
+    @test isapprox(counts,counts_golden;atol = 1e-12)
+
+    # TODO: Test AABB
+end
+