OSOceanAcoustics / echopype

Enabling interoperability and scalability in ocean sonar data analysis
https://echopype.readthedocs.io/
Apache License 2.0
94 stars 73 forks source link

Revise/subclass `SetGroups` #42

Closed leewujung closed 5 years ago

leewujung commented 5 years ago

Currently the class SetGroups under convert/set_nc_groups.py is written only for EK60 data and will likely error out when saving data unpacked from other echosounders.

We can just add switches in the current class to accommodate differences arising from different raw data formats, or we can write subclass for different formats.

@SvenGastauer @valentina-s: Thoughts?

SvenGastauer commented 5 years ago

I am not sure. I guess for EK60/EK80 switches would work because they are kind of similar, but thinking about other manufacturers and devices we will need subclasses at some point I'd guess, so we might as well just create them immediately for EK80 as well. I have a feeling that subclasses would be cleaner

leewujung commented 5 years ago

Yes, I agree subclasses would be cleaner too. Perhaps it makes more sense to keep only the nc groups that are simple in content (e.g., Top-level, Environment, and Provenance group) in the base class and move the current content for EK60 to a subclass also.

I think the EK80 raw data should be compatible with what's in the SONAR-netCDF4 convention. I use the data variable backscatter_r under the Beam group to store the raw power data from EK60. For EK80, the complex data can go into backscatter_r and backscatter_i if we follow the SONAR-netCDF4 convention. Although, I am wondering why not just store them in one complex data variable called backscatter as it seems more efficient that way?

Some additional note here which should probably go to a separate issue at some point:

The SONAR-netCDF4 convention is designed for just the raw data. Some fields are actually used to store the equations to calibrate the raw data. Therefore, I put the EK60 calibration routine under a separate submodule model (from model-view-controller, but it's probably better to be called "data model" -- I'll change that soon, otherwise it's easily confused with scattering models) and was thinking the pulse compression routines for EK80 can go in there too.

My hunch was that these operations require using ancillary parameters (such as the Sa parameter for EK60 and filters for EK80) that can be either read from the file or manipulated/input by the user, so it seems cleaner to split them out. @SvenGastauer : What do you think if we split out these components after you are happy with the EK80 functions?

leewujung commented 5 years ago

@ngkavin I'm just going to add you to this issue instead of writing a new once since this is under the same topic. We'll deal with the EK80 components later.

As we discussed, please:

leewujung commented 5 years ago

We'll deal with accommodating EK80 data when things are ready on that front.