handle edge case in reencode_samples

[ericphanson]

Encountered during testing in a closed-source repo.

Without this fix, we get

  Test threw exception
  Expression: OndaEDF.reencode_samples(samples) isa Samples
  InexactError: trunc(Int16, NaN)
  Stacktrace:
    [1] trunc
      @ ./float.jl:872 [inlined]
    [2] round
      @ ./float.jl:384 [inlined]
    [3] encode_sample
      @ ~/.julia/packages/Onda/7mYcu/src/samples.jl:192 [inlined]
    [4] encode_sample
      @ ~/.julia/packages/Onda/7mYcu/src/samples.jl:189 [inlined]
    [5] _broadcast_getindex_evalf
      @ ./broadcast.jl:683 [inlined]
    [6] _broadcast_getindex
      @ ./broadcast.jl:666 [inlined]
    [7] getindex
      @ ./broadcast.jl:610 [inlined]
    [8] macro expansion
      @ ./broadcast.jl:974 [inlined]
    [9] macro expansion
      @ ./simdloop.jl:77 [inlined]
   [10] copyto!
      @ ./broadcast.jl:973 [inlined]
   [11] copyto!
      @ ./broadcast.jl:926 [inlined]
   [12] materialize!(#unused#::Base.Broadcast.DefaultArrayStyle{2}, dest::Matrix{Int16}, bc::Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{2}, Nothing, typeof(Onda.encode_sample), Tuple{Base.RefValue{Type{Int16}}, Float64, Float64, Matrix{UInt64}}})
      @ Base.Broadcast ./broadcast.jl:884
   [13] materialize!
      @ ./broadcast.jl:881 [inlined]
   [14] broadcast!(::typeof(Onda.encode_sample), ::Matrix{Int16}, ::Type, ::Float64, ::Float64, ::Matrix{UInt64})
      @ Base.Broadcast ./broadcast.jl:850
   [15] encode!
      @ ~/.julia/packages/Onda/7mYcu/src/samples.jl:290 [inlined]
   [16] encode(::Type{Int16}, sample_resolution_in_unit::Float64, sample_offset_in_unit::Float64, sample_data::Matrix{UInt64}, dither_storage::Nothing)
      @ Onda ~/.julia/packages/Onda/7mYcu/src/samples.jl:252
   [17] encode(samples::Samples{Matrix{UInt64}}, dither_storage::Nothing)
      @ Onda ~/.julia/packages/Onda/7mYcu/src/samples.jl:322
   [18] encode
      @ ~/.julia/packages/Onda/7mYcu/src/samples.jl:321 [inlined]
   [19] reencode_samples(samples::Samples{Matrix{UInt64}}, sample_type::Type{Int16})
      @ OndaEDF ~/OndaEDF.jl/src/export_edf.jl:178
   [20] reencode_samples(samples::Samples{Matrix{UInt64}})
      @ OndaEDF ~/OndaEDF.jl/src/export_edf.jl:123
   [21] top-level scope
      @ ~/.julia/juliaup/julia-1.9.3+0.aarch64.apple.darwin14/share/julia/stdlib/v1.9/Test/src/Test.jl:478
   [22] eval
      @ ./boot.jl:370 [inlined]
   [23] eval_user_input(ast::Any, backend::REPL.REPLBackend, mod::Module)
      @ REPL ~/.julia/juliaup/julia-1.9.3+0.aarch64.apple.darwin14/share/julia/stdlib/v1.9/REPL/src/REPL.jl:153
   [24] repl_backend_loop(backend::REPL.REPLBackend, get_module::Function)
      @ REPL ~/.julia/juliaup/julia-1.9.3+0.aarch64.apple.darwin14/share/julia/stdlib/v1.9/REPL/src/REPL.jl:249
   [25] start_repl_backend(backend::REPL.REPLBackend, consumer::Any; get_module::Function)
      @ REPL ~/.julia/juliaup/julia-1.9.3+0.aarch64.apple.darwin14/share/julia/stdlib/v1.9/REPL/src/REPL.jl:234
   [26] run_repl(repl::REPL.AbstractREPL, consumer::Any; backend_on_current_task::Bool, backend::Any)
      @ REPL ~/.julia/juliaup/julia-1.9.3+0.aarch64.apple.darwin14/share/julia/stdlib/v1.9/REPL/src/REPL.jl:379
   [27] run_repl(repl::REPL.AbstractREPL, consumer::Any)
      @ REPL ~/.julia/juliaup/julia-1.9.3+0.aarch64.apple.darwin14/share/julia/stdlib/v1.9/REPL/src/REPL.jl:365
   [28] (::Base.var"#1017#1019"{Bool, Bool, Bool})(REPL::Module)
      @ Base ./client.jl:421
   [29] #invokelatest#2
      @ ./essentials.jl:819 [inlined]
   [30] invokelatest
      @ ./essentials.jl:816 [inlined]
   [31] run_main_repl(interactive::Bool, quiet::Bool, banner::Bool, history_file::Bool, color_set::Bool)
      @ Base ./client.jl:405
   [32] exec_options(opts::Base.JLOptions)
      @ Base ./client.jl:322
   [33] _start()
      @ Base ./client.jl:522

[kleinschmidt]

I'm a bit concerned that we're changing an error (albeit an inscrutable one) into a warning and creating the possibility of returning wrong results on EDF import for EDF files that are somehow corrupted.

I can see two ways of handling this a bit better. First would be to move the check introduced here to reencode_samples. Second, we could special case the "limiting" condition of dmin == dmax && smin == smax and set resolution to 1 there.

[ericphanson]

@kleinschmidt good points! In this case it is only smin == smax, dmin and dmax are generally fine (that would be a type-level issue, not data-level issue)

how about this?

[kleinschmidt]

better make sure the values are round-trippable...

[kleinschmidt]

phew looks good!