Skip to content

enh: add dimension alignment support for swap_dims_channel_frequency #1500

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

Closed
praneethratna wants to merge 3 commits into from
Closed

enh: add dimension alignment support for swap_dims_channel_frequency #1500

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
471f611
enh: add support for use frequency_nominal/channel as dimension
May 17, 2025
bb128fd
fix tests
May 17, 2025
cb17006
minor fix
May 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions echopype/consolidate/api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@

def swap_dims_channel_frequency(ds: Union[xr.Dataset, str, pathlib.Path]) -> xr.Dataset:
"""
Use frequency_nominal in place of channel to be dataset dimension and coorindate.
Use frequency_nominal in place of channel to be dataset dimension and coordinate.

This is useful because the nominal transducer frequencies are commonly used to
refer to data collected from a specific transducer.
Expand Down Expand Up @@ -349,6 +349,7 @@ def add_splitbeam_angle(
pulse_compression: bool = False,
storage_options: dict = {},
to_disk: bool = True,
use_frequency_nominal: bool = False,
) -> xr.Dataset:
"""
Add split-beam (alongship/athwartship) angles into the Sv dataset.
Expand Down Expand Up @@ -390,6 +391,9 @@ def add_splitbeam_angle(
If ``False``, ``to_disk`` with split-beam angles added will be returned.
``to_disk=True`` is useful when ``source_Sv`` is a path and
users only want to write the split-beam angle data to this path.
use_frequency_nominal: bool, default=False
If ``True``, the split-beam angles will be calculated using the
``frequency_nominal`` variable in ``source_Sv``.

Returns
-------
Expand Down Expand Up @@ -459,7 +463,11 @@ def add_splitbeam_angle(
if "channel" not in source_Sv.variables:
raise ValueError("The input source_Sv Dataset must have a channel dimension!")

# Select ds_beam channels from source_Sv
# Swap the dimension to frequency_nominal if use_frequency_nominal is True
if use_frequency_nominal:
source_Sv = swap_dims_channel_frequency(source_Sv)
Comment on lines 463 to +468
Copy link
Copy Markdown
Member

@leewujung leewujung May 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

What I was thinking was to make the add_splitbeam_angle function agnostic of whether in the input ds_Sv dataset has a dimension of channel or frequency_nominal. So the function will work with the original dimension (channel) straight from compute_Sv, and will also work if the dimension has already been changed to frequency_nominal before the add_splitbeam_angle function is called. So not running the swap_dims_channel_frequency under the hood within this function, but made this functions accepts ds_Sv with either dimensions.

Does the above make sense to you? So basically you can remove the added use_frequency_nominal argument and just have the function accepting both channel and frequency_nominal under the hood.


# Select ds_beam dimension from source_Sv
ds_beam = echodata[ed_beam_group].sel(channel=source_Sv["channel"].values)

# Assemble angle param dict
Expand Down
4 changes: 3 additions & 1 deletion echopype/consolidate/split_beam_angle.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,9 @@ def _e2f(angle_type: str) -> xr.Dataset:


def get_angle_complex_samples(
ds_beam: xr.Dataset, angle_params: dict, pc_params: dict = None
ds_beam: xr.Dataset,
angle_params: dict,
pc_params: dict = None,
) -> Tuple[xr.DataArray, xr.DataArray]:
"""
Obtain split-beam angle from CW or BB mode complex samples.
Expand Down
3 changes: 0 additions & 3 deletions echopype/test_data/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ Most of these files are stored on Git LFS but the ones that aren't (due to file
- 2019118 group2survey-D20191214-T081342.raw: Contains 6 channels but only 2 of those channels collect ping data
- D20200528-T125932.raw: Data collected from WBT mini (instead of WBT), from @emlynjdavies
- Green2.Survey2.FM.short.slow.-D20191004-T211557.raw: Contains 2-in-1 transducer, from @FletcherFT (reduced from 104.9 MB to 765 KB in test data updates)
- raw4-D20220514-T172704.raw: Contains RAW4 datagram, 1 channel only, from @cornejotux
- D20210330-T123857.raw: do not contain filter coefficients


### EA640
Expand All @@ -24,7 +22,6 @@ Most of these files are stored on Git LFS but the ones that aren't (due to file
- Winter2017-D20170115-T150122.raw: Contains a change of recording length in the middle of the file
- 2015843-D20151023-T190636.raw: Not used in tests but contains ranges are not constant across ping times
- SH1701_consecutive_files_w_range_change: Not used in tests. [Folder](https://drive.google.com/drive/u/1/folders/1PaDtL-xnG5EK3N3P1kGlXa5ub16Yic0f) on shared drive that contains sequential files with ranges that are not constant across ping times.
- NBP_B050N-D20180118-T090228.raw: split-beam setup without angle data
Copy link
Copy Markdown
Member

@leewujung leewujung May 22, 2025

Choose a reason for hiding this comment

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

I am not sure why these keep on happening (I found these on my local machine too. I think it might be due to me not setting up the docker container again and just working with the files I already have, so the new ones did not get pulled in). Could you please put these lines back? Thanks!



### AZFP
Expand Down
Loading