Alexander-Barth / NCDatasets.jl

Load and create NetCDF files in Julia
MIT License
146 stars 31 forks source link

Issues loading an inverted subset #262

Closed josuemtzmo closed 1 week ago

josuemtzmo commented 3 weeks ago

Describe the bug

Loading an inverted subset doesn't work, it outputs an error of illegal stride:

ds["temperature"][:,end:-1:1]

A few versions ago, this worked (I believed the version v0.13.2 allowed this operation)

The only way I manage to fix it is by extracting the array and then inverting it:

ds["temperature"][:,:][:,end:-1:1]

Isn't this a bit redundant?

To Reproduce

using NCDatasets
ds = NCDataset("file.nc")
ds["temperature"][:,end:-1:1]

Expected behavior

ds["temperature"][:,end:-1:1] should work.

Environment

julia> versioninfo()
Julia Version 1.10.4
Commit 48d4fd48430 (2024-06-04 10:41 UTC)
Build Info:
  Official https://julialang.org/ release
Platform Info:
  OS: Linux (x86_64-linux-gnu)
  CPU: 56 × Intel(R) Xeon(R) CPU E5-2680 v4 @ 2.40GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-15.0.7 (ORCJIT, broadwell)
Threads: 1 default, 0 interactive, 1 GC (on 56 virtual cores)
 pkg> status
  Status `~/github/single_eddy/Project.toml`
  [052768ef] CUDA v5.4.3
  [13f3f980] CairoMakie v0.12.7
  [85f8d34a] NCDatasets v0.14.4
  [9e8cae18] Oceananigans v0.91.8
  [d496a93d] SeawaterPolynomials v0.3.4

Full output

ds["temperature"][:,end:-1:1]
ERROR: NetCDF error: NetCDF: Illegal stride (NetCDF error code: -58)
Stacktrace:
 [1] check
   @ ~/.julia/packages/NCDatasets/JrRxr/src/errorhandling.jl:25 [inlined]
 [2] nc_get_vars!(ncid::Int32, varid::Int32, startp::Vector{…}, countp::Vector{…}, stridep::Vector{…}, ip::Array{…})
   @ NCDatasets ~/.julia/packages/NCDatasets/JrRxr/src/netcdf_c.jl:1028
 [3] _read_data_from_nc!(::NCDatasets.Variable{…}, ::Array{…}, ::Base.OneTo{…}, ::Vararg{…})
   @ NCDatasets ~/.julia/packages/NCDatasets/JrRxr/src/variable.jl:412
 [4] readblock!(::NCDatasets.Variable{…}, ::Array{…}, ::Base.OneTo{…}, ::Vararg{…})
   @ NCDatasets ~/.julia/packages/NCDatasets/JrRxr/src/variable.jl:395
 [5] getindex_disk(::NCDatasets.Variable{Float64, 3, NCDataset{Nothing, Missing}}, ::Function, ::Vararg{Any})
   @ DiskArrays ~/.julia/packages/DiskArrays/bZBJE/src/diskarray.jl:40
 [6] getindex
   @ ~/.julia/packages/DiskArrays/bZBJE/src/diskarray.jl:211 [inlined]
 [7] getindex(::CommonDataModel.CFVariable{…}, ::Colon, ::Colon, ::StepRange{…})
   @ CommonDataModel ~/.julia/packages/CommonDataModel/G3moc/src/cfvariable.jl:445
 [8] top-level scope
   @ REPL[23]:1
Some type information was truncated. Use `show(err)` to see complete types.
ctroupin commented 3 weeks ago

Hello,

thanks for reporting this bug, which I could reproduce on my machine.

While I don't have the fix to the issue, I wanted to suggest to test, instead of

ds["temperature"][:,:][:,end:-1:1]

the function reverse(), so in this case it would be

T = reverse(ds["temperature"][:,:], dims=2)
Alexander-Barth commented 2 weeks ago

Hi Josué, thank you for your detailed and reproducible bug report. I just committed this fix 968ebbf to allow negative strides.

Do you have the time to test this for your use case, before I make a new release?

ctroupin commented 2 weeks ago

I've checked with the last commit and the code was now working with negative strides.

josuemtzmo commented 2 weeks ago

Apologies for the delay, I have checked with the last commit and the code works with negative strides! Thank you so much!!

Alexander-Barth commented 1 week ago

Thanks you for confirming!