CRIMAC-WP4-Machine-learning / CRIMAC-Raw-To-Svf-TSf

Python code showing the current processing steps for modern broadband echosounders, from raw data to Sv(f) and TS(f).
GNU Lesser General Public License v3.0
5 stars 1 forks source link

Review of code and comparison to paper (chapter II A-D) #1

Open nilsolav opened 2 years ago

nilsolav commented 2 years ago

I started reviewing the paper and comparing to the code.

In the example ekcalc = EK80Calculation('./data/pyEcholabEK80data.json') the result has field ekcalc.f_s. In the code the f_s is not consistently used. Some places uses fs. Should we not either use Tex formatting throughout or not?

Is the ekcalc.y_rx_org the decimated data stored in the raw files? If this comes from the raw files is this not y_rx(n,u)? The y_rx_org seem to be complex (from the filter bank) and passed directly to the pulse compression function, and not the data prior to the filter bank, as defined in the paper. If this is correct we should use y_rx_nu instead of y_rx_org. for consistency we should also use y_rx_org_nu instead of y_rx_org.

The json file has this field: "Raw file": "C:/Users/a32685/Documents/Projects/2020_CRIMAC/CRIMACHackEx/cal-babak-D20201120-T080856.raw",. should we remove that? The json files also has names associated with LSSS and pyEcholab. Do we not want to make this independent of these packages/software solutions?

I think we should use y_pc_nu instead of y_pc_u for consistency when referring to $y_pc(n,u)$.

Again, for consistency y_mf_auto -> y_mf_auto_n.

nilsolav commented 2 years ago

I created a branch called ChapterII where I am comparing the paper and the code. I made the following changes: [Note from Nils Olav: There are some inconsistencies in the variable naming: ytx0 -> y_tx_n ytx0_t -> y_tx_n_t y_tx -> y_tx_n y_tx -> y_tilde_tx_n (anbigous) Introduced y_tilde_tx_nv for filter stages y_mf -> y_mf_n Introduced y_mf_n_conj_rev

I have not adjsuted the function that depends on this down stream.

nilsolav commented 2 years ago

Branch: chapterII Comit: [47d4465] Changed y_rx_org -> y_rx_nu and y_mf_n -> y_mf_n_conj_rev to conform with the paper. Please review the latter.