jpjones76 / SeisIO.jl

Julia language support for geophysical time series data
http://seisio.readthedocs.org
Other
47 stars 21 forks source link

get_data fails due to unsafe_copyto! type mismatch #43

Closed tclements closed 4 years ago

tclements commented 4 years ago

I'm doing a sizeable download (500+ stations, list attached) using get_data.

I'm using SeisIO v1.0.0 and Julia 1.4, this is also fails on master. Code to reproduce:

using SeisIO, Dates, DelimitedFiles
stationlist = readdlm("stationlist.txt",String)[:]
startdate = DateTime(2019,1,1)
enddate = DateTime(2019,1,4)
get_data("FDSN",stationlist,s=startdate,t=enddate+Day(1),w=true,v=2)

This fails due to unsage_copyto! trying to copy Float32 data to a Float64 array. Looks like x is a Float64 array while getfield(BUF, :x) is a Float32 array at line 265 in 2_parserec.jl.

ERROR: MethodError: no method matching unsafe_copyto!(::Array{Float64,1}, ::Int64, ::Array{Float32,1}, ::Int64, ::Int64)
Closest candidates are:
  unsafe_copyto!(::Array{T,N} where N, ::Any, ::Array{T,N} where N, ::Any, ::Any) where T at array.jl:263
  unsafe_copyto!(::BitArray, ::Integer, ::Union{BitArray, Array}, ::Integer, ::Integer) at bitarray.jl:452
  unsafe_copyto!(::CuArrays.CuArray{T,N,P} where P where N, ::Any, ::Array{T,N} where N, ::Any, ::Any) where T at /home/timclements/.julia/packages/CuArrays/4Q1BY/src/array.jl:300
  ...
Stacktrace:
 [1] parserec!(::SeisData, ::SeisIO.SeisIOBuf, ::IOStream, ::Int64, ::Int64, ::Bool, ::Int64) at /home/timclements/.julia/packages/SeisIO/pSAug/src/Submodules/SEED/2_parserec.jl:265
 [2] parsemseed!(::SeisData, ::IOStream, ::Int64, ::Int64, ::Bool, ::Int64) at /home/timclements/.julia/packages/SeisIO/pSAug/src/Submodules/SEED/readmseed.jl:11
 [3] FDSNget!(::SeisData, ::Array{String,2}; autoname::Bool, fmt::String, msr::Bool, nd::Int64, opts::String, rad::Array{Float64,1}, reg::Array{Float64,1}, s::DateTime, si::Bool, src::String, t::DateTime, to::Int64, v::Int64, w::Bool, xf::String, y::Bool) at /home/timclements/.julia/packages/SeisIO/pSAug/src/Web/FDSN.jl:260
 [4] get_data(::String, ::Array{String,1}; autoname::Bool, demean::Bool, detrend::Bool, fmt::String, msr::Bool, nd::Int64, opts::String, rad::Array{Float64,1}, reg::Array{Float64,1}, prune::Bool, rr::Bool, s::DateTime, si::Bool, src::String, taper::Bool, t::DateTime, to::Int64, ungap::Bool, unscale::Bool, v::Int64, w::Bool, xf::String, y::Bool) at /home/timclements/.julia/packages/SeisIO/pSAug/src/Wrappers/get_data.jl:60
 [5] top-level scope at none:0

I don't know which station this is failing for.

stationlist.txt

jpjones76 commented 4 years ago

Checking now.

jpjones76 commented 4 years ago

The error is thrown on the first packet from C0.HAYD.00.LHZ. From your MWE, I can reproduce with: get_data("FDSN",stationlist[155:155],s=startdate,t=enddate+Day(1),v=3)

The issue is that the mini-SEED parser implicitly assumes a Float32 data vector in each channel of a SeisData structure. I don't think I should be assuming that, though.

Three potential solutions. Any preference?

I'll be able to tell you how much each solution affects performance in a bit. Testing and benchmarking.

tclements commented 4 years ago

I'm thinking either placing this check in parserec! or parsemseed! would be preferable. I expect the performance overhead for a call to eltype or sizeof would be on the order of nanoseconds per blockette.

jpjones76 commented 4 years ago

This will sound strange, but I find no appreciable performance change if I change unsafe_copyto! to copyto. Should I benchmark the other two ideas, or just use this solution?

tclements commented 4 years ago

Going with copyto sounds good to me!

jpjones76 commented 4 years ago

Sorry for the delay on this. My tests are running into an unrelated issue with IRIS timeseries requests and I'm waiting for resolution before pushing to master.

jpjones76 commented 4 years ago

Okay, IRIS has confirmed that the IRISWS timeseries issue is unrelated, but I need to make a few test changes before I can push this live (otherwise my automated tests fail). Hopefully this is all fixed tonight.

jpjones76 commented 4 years ago

I've finally found the change to IRISWS timeseries that was breaking my tests, so commits are getting pushed to master again. The change that fixes this issue should now be fixed on the master branch.

jpjones76 commented 4 years ago

Made one final change due to a weird bug that showed up while investigating this. Everything related to this issue should be fixed on master. If you're using only release versions, you can expect a minor version increment with these fixes in it later this week.