Ocean contour updates

[hugo-sardi]

As mentioned over email in multiple conversations, here are some updates related to the OceanContour parser. The changes here include bin mapping support and you can test that with this file (remove the .txt extension) : S101916A007_USG_SIG1000_avgd.ad2cp.txt

What is missing here is the primary test to call the docstrings or test individual components and files. I think there are two options here for devs: A. improve the docstring parser to be able to parse multiple docstrings in a single source file, or B. write a unit test file that will test the individual components/parser options/etc.

[lbesnard]

@hugo-sardi any chance you could upload the .netcdf as I don't have ocean contour software, so that PR can be tested and unittests included since the parser only takes NetCDF or MAT files as input.

I'm trying to get my head around this OceanContour parser. The main issue I have for now is that I can't replicated your docstring tests:

            % Example:
            %
            % %read from netcdf
            % file = [toolboxRootPath 'data/testfiles/netcdf/Nortek/OceanContour/Signature/s500_enu_avg.nc'];
            % [sample_data] = OceanContour.readOceanContourFile(file);
            % assert(strcmpi(sample_data{1}.meta.instrument_model,'Signature500'))
            % assert(isequal(sample_data{1}.meta.instrument_avg_interval,60))
            % assert(isequal(sample_data{1}.meta.instrument_sample_interval,600))
            % assert(strcmpi(sample_data{1}.meta.coordinate_system,'ENU'))
            % assert(isequal(sample_data{1}.meta.nBeams,4))
            % assert(strcmpi(sample_data{1}.dimensions{2}.name,'HEIGHT_ABOVE_SENSOR'))
            % assert(~isempty(sample_data{1}.variables{end}.data))
            %
            % % read from matfile
            % file = [toolboxRootPath 'data/testfiles/mat/Nortek/OceanContour/Signature/s500_enu_avg.mat'];
            % [sample_data] = OceanContour.readOceanContourFile(file);
            % assert(strcmpi(sample_data{1}.meta.instrument_model,'Signature500'))
            % assert(isequal(sample_data{1}.meta.instrument_avg_interval,60))
            % assert(isequal(sample_data{1}.meta.instrument_sample_interval,600))
            % assert(strcmpi(sample_data{1}.meta.coordinate_system,'ENU'))
            % assert(isequal(sample_data{1}.meta.nBeams,4))
            % assert(strcmpi(sample_data{1}.dimensions{2}.name,'HEIGHT_ABOVE_SENSOR'))
            % assert(~isempty(sample_data{1}.variables{end}.data))
            %
            %
            % author: hugo.oliveira@utas.edu.au

(note that I have the .mat file you send me back in June)

running the above test on the NetCDF or MAT file leads to this:

file =

    'imos-toolbox/data/testfiles/netcdf/Nortek/OceanContour/Signature/s500_enu_avg.nc'

>> [sample_data] = OceanContour.readOceanContourFile(file)
Reference to non-existent field 'DataInfo_transformsAndCorrections_binMapping'.

Error in OceanContour>@(x)(file_metadata.(att_mapping.(x))) (line 530)
                get_att = @(x)(file_metadata.(att_mapping.(x)));

Error in OceanContour.readOceanContourFile (line 551)
                meta.binMapping = get_att('binMapping');

Were your tests working when you develop this parser?

[hugo-sardi]

@lbesnard

the NetCDF generated by OceanContour from the ad2cp file I posted above can be found here:

https://public-sardi-oceanography.s3.ap-southeast-2.amazonaws.com/misc/SAMUSG_2021_09/Exported+Data/S101916A007_USG_SIG1000_avgd/Avg_001.VTC.nc

I recommend testing against this one and creating a new unit-test case. I would also recommend getting the OceanContour license since more development will likely pop up (e.g. waves, other options/extra attributes, etc).

Were your tests working when you develop this parser?

Yes - the parser was developed some time ago and based initially on a non-bin-mapped file. However, the following changes here assumed BinMapping was consistently applied, as you found out. I obviously didn't test it against the older files in this PR.

The fix is quite simple though - just wrap the assignment in a try stateemnt and set the binmapped variable as false otherwise:

try
    meta.binMapping = get_att('binMapping');
    binmapped = logical(meta.BinMapping);
catch
    binmapped = false
end

[lbesnard]

I recommend testing against this one and creating a new unit-test case. I would also recommend getting the OceanContour license since more development will likely pop up (e.g. waves, other options/extra attributes, etc).

I don't think it's going to happen any time soon!

So regarding your Avg_001.VTC.nc , at the moment it can't be imported to the Ocean Contour parser as is, and I don't have access to any working files to test the parser without having to do any code changes.

I noticed that some variable names start with BinMap such as BinMapAvgVelocityENU_Range. This breaks the variable mapping witch maps AvgVelocityENU_Range to HEIGHT_ABOVE_SENSOR for example

So is this BinMap prefix was added by OceanContour and supposed to be here? This would mean that it should be directly mapped to HEIGHT_ABOVE_SENSOR which is currently not handled by the parser. or was this prefix added by you and should be removed from the NetCDF?

[hugo-sardi]

So regarding your Avg_001.VTC.nc , at the moment it can't be imported to the Ocean Contour parser as is, and I don't have access to any working files to test the parser without having to do any code changes.

Are you sure? What is the error you found? I'm using these bits of code for quite a while now.

I noticed that some variable names start with BinMap such as BinMapAvgVelocityENU_Range. This breaks the variable mapping witch maps AvgVelocityENU_Range to HEIGHT_ABOVE_SENSOR for example

No it doesn't break since the handling of dimension association/name is done by inspecting meta.coordinate_system (see line ~684). FYI, the prefix is dynamic (in this case BinMap, otherwise it is an empty string).

So is this BinMap prefix was added by OceanContour and supposed to be here?

Yes.

This would mean that it should be directly mapped to HEIGHT_ABOVE_SENSOR which is currently not handled by the parser.

As said above, check lines ~660-707.

or was this prefix added by you and should be removed from the NetCDF?

No. You can't change that - this is the way oceancontour creates the variables if binmapping was selected in the software. Hence, dynamic prefix logic handling is a necessary part of the parser.

[evacougnon]

Closing as superseded by #793