Skip to content

discretediag's signature should imo be restricted to Integer #156

@penelopeysm

Description

@penelopeysm

function discretediag(
chains::AbstractArray{<:Real,3}; frac::Real=0.3, method::Symbol=:weiss, nsim::Int=1000
)

The type AbstractArray{3,Real} here is too broad: inside the helper function discretediag_sub, it immediately gets converted to Int anyway.

for j in 1:num_vars
X = convert(AbstractMatrix{Int}, c[:, :, j])
result = diag_all(X, method, nsim, start_iter, step_size)

Technically, there isn't any wrong behaviour going on here because non-integer data will error, but it is probably a more confusing error than necessary (see below). It would probably be shorter and easier to parse if it was a MethodError saying that Array{3,Float64} is not accepted, available methods are Array{3,Integer}.

If the motivation for the signature is to work around MCMCChains's overly aggressive conversion to Float, then I think a better place to handle that would be the MCMCChains overload here?

Said stack trace:

ERROR: InexactError: Int64(-0.2146900723338993)
Stacktrace:
  [1] Int64
    @ ./float.jl:994 [inlined]
  [2] convert
    @ ./number.jl:7 [inlined]
  [3] setindex!
    @ ./genericmemory.jl:243 [inlined]
  [4] unsafe_copyto!(dest::Memory{Int64}, doffs::Int64, src::Memory{Float64}, soffs::Int64, n::Int64)
    @ Base ./genericmemory.jl:153
  [5] unsafe_copyto!
    @ ./genericmemory.jl:133 [inlined]
  [6] _copyto_impl!
    @ ./array.jl:308 [inlined]
  [7] copyto!
    @ ./array.jl:294 [inlined]
  [8] copyto!
    @ ./array.jl:319 [inlined]
  [9] copyto_axcheck!
    @ ./abstractarray.jl:1167 [inlined]
 [10] AbstractMatrix{Int64}(A::Matrix{Float64})
    @ Base ./array.jl:627
 [11] convert
    @ ./abstractarray.jl:18 [inlined]
 [12] discretediag_sub(c::PermutedDimsArray{…}, frac::Float64, method::Symbol, nsim::Int64, start_iter::Int64, step_size::Int64)
    @ MCMCDiagnosticTools ~/.julia/packages/MCMCDiagnosticTools/wE49x/src/discretediag.jl:388
 [13] discretediag(chains::PermutedDimsArray{…}; frac::Float64, method::Symbol, nsim::Int64)
    @ MCMCDiagnosticTools ~/.julia/packages/MCMCDiagnosticTools/wE49x/src/discretediag.jl:448
 [14] discretediag(chains::PermutedDimsArray{Float64, 3, (1, 3, 2), (1, 3, 2), Array{Float64, 3}})
    @ MCMCDiagnosticTools ~/.julia/packages/MCMCDiagnosticTools/wE49x/src/discretediag.jl:438
 [15] discretediag(chains::Chains{…}; sections::Symbol, kwargs::@Kwargs{})
    @ MCMCChains ~/.julia/packages/MCMCChains/vqJuv/src/discretediag.jl:16
 [16] discretediag(chains::Chains{Float64, AxisArrays.AxisArray{…}, Missing, @NamedTuple{…}, @NamedTuple{…}})
    @ MCMCChains ~/.julia/packages/MCMCChains/vqJuv/src/discretediag.jl:7
 [17] top-level scope
    @ REPL[14]:1
Some type information was truncated. Use `show(err)` to see complete types.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions