Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NCODV.load failing if quality flags not defined #92

Closed
1 of 2 tasks
ctroupin opened this issue Dec 9, 2021 · 7 comments
Closed
1 of 2 tasks

NCODV.load failing if quality flags not defined #92

ctroupin opened this issue Dec 9, 2021 · 7 comments

Comments

@ctroupin
Copy link
Member

ctroupin commented Dec 9, 2021

I'm trying to read an ODV netCDF but it fails with this message:

julia> NCODV.load(Float64, inputfile, varlist[1])
ERROR: type Nothing has no field var
Stacktrace:
 [1] getproperty(x::Nothing, f::Symbol)
   @ Base ./Base.jl:42
 [2] (::DIVAnd.NCODV.var"#10#11"{Int64, DataType, String, String, Vector{String}})(ds::NCDataset{Nothing})
   @ DIVAnd.NCODV ~/.julia/packages/DIVAnd/eDBbE/src/NCODV.jl:336
 [3] NCDataset(f::DIVAnd.NCODV.var"#10#11"{Int64, DataType, String, String, Vector{String}}, args::String; kwargs::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
   @ NCDatasets ~/.julia/packages/NCDatasets/opjna/src/dataset.jl:320
 [4] NCDataset(f::Function, args::String)
   @ NCDatasets ~/.julia/packages/NCDatasets/opjna/src/dataset.jl:318
 [5] #load#9
   @ ~/.julia/packages/DIVAnd/eDBbE/src/NCODV.jl:281 [inlined]
 [6] load(T::Type, fname::String, long_name::String)
   @ DIVAnd.NCODV ~/.julia/packages/DIVAnd/eDBbE/src/NCODV.jl:279
 [7] top-level scope
   @ REPL[13]:1

It seems it is because the variables storing the quality flags were not defined:
https://github.com/gher-ulg/DIVAnd.jl/blob/master/src/NCODV.jl#L336

The question: shall we

  • allow for netCDF files without quality flags (hence modifying the load and loadprof sections
    or
  • add a line in the docstring of the function to state that the quality flags must be defined?
@jmbeckers
Copy link
Member

I would to everything to make the life of the users simple. So allow for netCDF files without flags, put out a warning and fill in the quality values by default to good and then proceed as usual

@Alexander-Barth
Copy link
Member

I think we have a document somewhere describing how the NetCDF from ODV need to be prepared. Can we include this in the documentation of this function ?

Note that this loop (https://github.com/gher-ulg/DIVAnd.jl/blob/master/src/NCODV.jl#L99) is quite performance critical. We need to be sure that for the common case (flags present), the code is not slower. @ctroupin , can you make a small NetCDF files producing this error ?

@ctroupin
Copy link
Member Author

Yes, the preparation of the file is described step-by-step here:
https://github.com/gher-ulg/Diva-Workshops/blob/master/tricks/ODV_netCDF_export.md
and we indeed ask the user to export the quality flags, so the issue should not happen (it did happen because of some specific tests carried out for Reiner).

I agree with @jmbeckers: making the life of the users simple is certainly the good choice. So we can keep this issue as a possible enhancement for the future (maybe not prioritary).

Here is a small file: https://dox.ulg.ac.be/index.php/s/nGmsH5ydCAjgft5

@Alexander-Barth
Copy link
Member

Thanks @ctroupin for the small file. My branch can now read the file with a warning. Can you check the master branch and my branch with a large ODV files (with all the flags) to check if there is a slow down (for what should be the common case)?

@ctroupin
Copy link
Member Author

ok I'll check that, I have large files that wait to be tested!

@ctroupin
Copy link
Member Author

First results: reading a large netCDF ODV file (with all the flags and metadata) takes:

  • [master]: 904.8 s (33.50 M allocations: 7.140 GiB, 0.28% gc time, 0.41% compilation time)
  • [Alex:] 947.9 s (25.85 M allocations: 4.749 GiB, 0.18% gc time).

@Alexander-Barth
Copy link
Member

Thanks for the info. I guess this is the price to pay for making the code more general/complex. (If somebody wants to try a different approach, feel free to test it).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants