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.

Sign up for GitHub

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

Jump to bottom

Refactor split-beam processing and add BB with pc option #971

Merged
merged 84 commits into from
Mar 11, 2023
Merged

Refactor split-beam processing and add BB with pc option #971

merged 84 commits into from
Mar 11, 2023

Conversation

leewujung
Copy link
Member

@leewujung leewujung commented Mar 10, 2023 •
edited
Loading

Overview

This PR completely refactors the split-beam processing code and addresses #890.

Details

Immediately related to split-beam processing

  • consolidate complex sample split-beam processing to a single function get_angle_complex_samples, which:
    • handles both BB and CW cases
    • handles both with and without pulse compression cases
  • fix bugs in determining whether to pass in all or single channels for split-beam processing based on beam_type

Others

  • add angle_sensitivity_* to CAL_PARAMS
  • add BB freq_center re frequency_nominal scaling for angle_sensitivity_* and beamwidth_* in cal params construction
  • move get_filter_coeff() and get_vend_filter_EK80() to ek80_complex.py
    • this allows using pulse compression in split-beam BB complex with pc option
  • change compress_pulse to output xr.DataArray instead of xr.Dataset
  • rename test_consolidate.py to test_consolidate_integration.py
  • overhaul docstring

Note this built on top of #955 so need to pull in dev once that is merged.

@leewujung
add some initial sketch on the changes
e221c46
@leewujung
draft up cal_params.sanitize_user_cal_dict and param_dict2da
7763e48
@leewujung
draft up get_cal_params_EK_new and _test_interp_da to handle all tria…
6ac16cb 
…ge and interpolating of cal params
@leewujung
add test_get_interp_da and placeholder for other cal_param tests
ade210a
@leewujung
add pre-commit fixes
8318ee5
@leewujung
add test for sanitize_user_cal_dict
39fc762
@leewujung
tidy up comments
13a38c1
@leewujung
revise/rename param2da for organizing individual param, add test
2763548
@leewujung
add param2da use to sanitize_user_cal_dict, add corresponding tests
b2fba5e
@leewujung
fix logic in get_cal_params_EK_new and clearly lay out alternative da…
14fd15a 
…ta type in _get_interp_da
@leewujung
revise default cal/env_params handling in CalibrateBase.__init__, sti…
837c38d 
…ll need to test these
@leewujung
change CalibrateEK80._cal_complex_samples to use params from self.cal…
c757c1d 
…_params; still need to do the same for CalibrateEK60 and CalibrateAZFP
@leewujung
udpate tests
77ad48b
@leewujung
add tests for only subset of channel needs to be interpolated
beb1388
@leewujung
fix a few more things in _get_intep_da for EK80 CW mode data; note do…
d4ad524 
…wn new tests needed
@leewujung
minor tweak to use get_cal_params_EK_new for calibrating EK60
500c696
@leewujung
remove the old get_cal_params_EK
d576b31
@leewujung
completely remove old private functions in CalibrateEK80, run pre-commit
f70f240
@leewujung
revise get_cal_params_AZFP to use sanitize_user_cal_dict
353d3f7
@leewujung
squeeze and drop beam dimension earlier in CalibrateAZFP._cal_power_s…
6f86888 
…amples
@leewujung
pre-commit changes
0a5d6dd
@leewujung
fix docstring, pre-commit changes
1ef5335
@leewujung
rename get_cal_params_EK_new to get_cal_params_EK
9fc30ab
@leewujung
Merge branch 'cal-intake-input' into cal-intake
27566b1
@leewujung
add test_get_cal_params_AZFP
edcdd6e
@leewujung
add da name to individual param data in sanitize_user_cal_dict
c553e3e
@leewujung
allow coord combination (cal_channel_id, cal_frequency) in sanitize_u…
3124cd7 
…ser_cal_dict
@leewujung
add interp for user-input freq-dependent param values, add tests
913b16d
@leewujung
Merge branch 'cal-intake-input' into cal-intake
5673fb4
@leewujung
add test_get_cal_params_EK60, change sanitize_user_cal_dict handling …
feb72f8 
…of EK types
leewujung and others added 12 commits March 11, 2023 02:57
@leewujung @lsetiawan
use f string in place of %
4c4f3c0 
Co-authored-by: Don Setiawan <landungs@uw.edu>
@leewujung @lsetiawan
use f string in place of %
775f666 
Co-authored-by: Don Setiawan <landungs@uw.edu>
@leewujung @lsetiawan
use f string in place of %
a4a3a90 
Co-authored-by: Don Setiawan <landungs@uw.edu>
@leewujung @lsetiawan
add input argument typing for get_filter_coeff
5a50212 
Co-authored-by: Don Setiawan <landungs@uw.edu>
@leewujung @lsetiawan
add parameters docstring for get_filter_coeff
1d1b84b 
Co-authored-by: Don Setiawan <landungs@uw.edu>
@pre-commit-ci
[pre-commit.ci] auto fixes from pre-commit.com hooks
8535655 
for more information, see https://pre-commit.ci
@leewujung @lsetiawan
change from .data to .values
3784a63 
Co-authored-by: Don Setiawan <landungs@uw.edu>
@leewujung @lsetiawan
change from .data to .values
358ee23 
Co-authored-by: Don Setiawan <landungs@uw.edu>
@leewujung
Merge branch 'fix-split-beam' of https://github.com/leewujung/echopype …
161d6d5 
…into fix-split-beam
@leewujung
remove outdated NotImplementedError in get_angle_power_samples
7809ebc
@leewujung
add Literal to ek80_complex.py import
a8706ff
@pre-commit-ci
[pre-commit.ci] auto fixes from pre-commit.com hooks
66b1ac9 
for more information, see https://pre-commit.ci
lsetiawan
lsetiawan approved these changes Mar 10, 2023
View reviewed changes
Copy link
Member

@lsetiawan lsetiawan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

leewujung and others added 13 commits March 10, 2023 16:28
@leewujung
add TODO
8a923da
@leewujung @lsetiawan
improve get_filter_coeff docstring
8ebed6d 
Co-authored-by: Don Setiawan <landungs@uw.edu>
@leewujung
Merge branch 'fix-split-beam' of https://github.com/leewujung/echopype …
e1e353a 
…into fix-split-beam
@leewujung @lsetiawan
fix typo
97d5c7b 
Co-authored-by: Don Setiawan <landungs@uw.edu>
@leewujung @lsetiawan
improve get_angle_power_samples docstring
a635d28 
Co-authored-by: Don Setiawan <landungs@uw.edu>
@leewujung
Merge branch 'fix-split-beam' of https://github.com/leewujung/echopype …
a899131 
…into fix-split-beam
@leewujung @lsetiawan
improve get_angle_complex_samples docstring
f9b1fad 
Co-authored-by: Don Setiawan <landungs@uw.edu>
@leewujung
Merge branch 'fix-split-beam' of https://github.com/leewujung/echopype …
5cac799 
…into fix-split-beam
@leewujung
add return to compress_pulse docstring
0a4e6be
@leewujung
Merge branch 'dev' into fix-split-beam
8557e7f
@leewujung
break long lines
235bc4c
@leewujung
remove beam dim checking for theta and phi in get_angle_complex_sampl…
00e35ab 
…e, keep beam dim removal in get_angle_power_sample
@leewujung
add test placeholder for EK80 CW power angle
cfa31a5
@leewujung
Copy link
Member Author

Found a missing test case for EK80 CW power data. I am pretty sure the code works right now, but don't have Echoview dongle to produce a comparison dataset. Will get this PR merged and add this later.

@leewujung leewujung merged commit c1be8c8 into OSOceanAcoustics:dev Mar 11, 2023
This was referenced Mar 13, 2023
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lsetiawan lsetiawan lsetiawan approved these changes

Assignees

@leewujung leewujung

Labels
processing functions
Projects
Echopype
Status: Done
Milestone
0.6.4
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@leewujung @codecov-commenter @lsetiawan