-
Notifications
You must be signed in to change notification settings - Fork 98
Add functions for broadband TS spectrum #1664
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
b057324
c6886e3
38a012e
2aed1db
5f05da1
fe4adec
47ad17d
b2ff5a2
31d401b
c8c657a
263f78b
2b9db6b
7e845bc
7fe30d4
68af1bb
c905d3e
c51e936
78926cc
8bc448b
f386382
33ae025
526f36f
7f88c4c
5b18b2c
b058ac3
78da373
884567c
509e709
be3ed86
cb6c9e3
d5a71bb
99eb722
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,3 @@ | ||
| from .api import compute_Sv, compute_TS | ||
| from .api import compute_Sp, compute_Sv, compute_Sv_spectrum, compute_TS, compute_TS_spectrum | ||
|
|
||
| __all__ = ["compute_Sv", "compute_TS"] | ||
| __all__ = ["compute_Sv", "compute_TS", "compute_Sp", "compute_Sv_spectrum", "compute_TS_spectrum"] |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,3 +1,5 @@ | ||||||
| import warnings | ||||||
|
|
||||||
| import numpy as np | ||||||
| import xarray as xr | ||||||
|
|
||||||
|
|
@@ -23,7 +25,7 @@ | |||||
|
|
||||||
|
|
||||||
| def _compute_cal( | ||||||
| cal_type, | ||||||
| cal_type: str, | ||||||
| echodata: EchoData, | ||||||
| env_params=None, | ||||||
| cal_params=None, | ||||||
|
|
@@ -32,9 +34,20 @@ def _compute_cal( | |||||
| encode_mode=None, | ||||||
| assume_single_filter_time=None, | ||||||
| drop_last_hanning_zero=False, | ||||||
| **kwargs, | ||||||
| ): | ||||||
| # Make waveform_mode "FM" equivalent to "BB" | ||||||
| waveform_mode = "BB" if waveform_mode == "FM" else waveform_mode | ||||||
| # Make waveform_mode "FM" equivalent to "BB". | ||||||
| # Accept legacy "BB" for backward compatibility. | ||||||
| # Ref: https://github.com/echostack-org/echopype/issues/1651 | ||||||
| if waveform_mode == "BB": | ||||||
| warnings.warn( | ||||||
| "'BB' is deprecated and will be removed in a future release. " | ||||||
| "Please use 'FM' instead.", | ||||||
| DeprecationWarning, | ||||||
| stacklevel=2, | ||||||
| ) | ||||||
|
|
||||||
| waveform_mode = "BB" if waveform_mode in ("FM", "BB") else waveform_mode | ||||||
|
|
||||||
| # TODO: consolidate the below block with simrad.py::check_input_args_combination() | ||||||
| # Check on waveform_mode, encode_mode inputs, and assumption on single filter time | ||||||
|
|
@@ -82,13 +95,27 @@ def _compute_cal_ds(echodata, slice_dict): | |||||
| # Check Echodata backscatter data size and recommend chunking if data is too large | ||||||
| cal_obj._check_echodata_backscatter_size() | ||||||
|
|
||||||
| # Perform calibration | ||||||
| if cal_type == "Sv": | ||||||
| cal_ds = cal_obj.compute_Sv() | ||||||
| else: | ||||||
| cal_ds = cal_obj.compute_TS() | ||||||
| compute_methods = { | ||||||
| "Sp": "compute_Sp", | ||||||
| "TS": "compute_TS", | ||||||
| "Sv": "compute_Sv", | ||||||
| # add Sp_spectrum?? | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes I think there should be updates: from our chat sounds like they will get added after the single target detection PR gets updated (since it's tied to this one in some specific aspects)? |
||||||
| "TS_spectrum": "compute_TS_spectrum", | ||||||
| } | ||||||
|
|
||||||
| return cal_ds | ||||||
| try: | ||||||
| method_name = compute_methods[cal_type] | ||||||
| except KeyError: | ||||||
| raise ValueError(f"Unsupported calibration type: {cal_type}") from None | ||||||
|
|
||||||
| compute_method = getattr(cal_obj, method_name, None) | ||||||
|
|
||||||
| if compute_method is None: | ||||||
| raise ValueError( | ||||||
| f"{cal_type} calibration is not supported for " f"{echodata.sonar_model} data." | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Comment on lines
+106
to
+115
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not following the logic here - wouldn't it be better to just do a check if And then there's a second Maybe these can be consolidated to something simpler, that checks if |
||||||
| ) | ||||||
|
|
||||||
| return compute_method(**kwargs) | ||||||
|
|
||||||
| # Calibrate as a single dataset if not Ex80 | ||||||
| model_family = SONAR_MODELS[echodata.sonar_model]["family"] | ||||||
|
|
@@ -164,7 +191,10 @@ def _compute_cal_ds(echodata, slice_dict): | |||||
|
|
||||||
| # Calibrate and drop filter_time | ||||||
| cal_ds_iteration = _compute_cal_ds(echodata, slice_dict) | ||||||
| cal_ds_list.append(cal_ds_iteration.drop_vars("filter_time")) | ||||||
| if "filter_time" in cal_ds_iteration: | ||||||
| cal_ds_iteration = cal_ds_iteration.drop_vars("filter_time") | ||||||
|
Comment on lines
+194
to
+195
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What are the cases that
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe this is related to #1653? |
||||||
|
|
||||||
| cal_ds_list.append(cal_ds_iteration) | ||||||
|
|
||||||
| # # Alternative? | ||||||
| # for channel in echodata[ed_beam_group]["channel"].values: | ||||||
|
|
@@ -204,12 +234,27 @@ def _add_attrs(cal_type, ds): | |||||
| """Add attributes to backscattering strength dataset. | ||||||
| cal_type: Sv or TS | ||||||
| """ | ||||||
| ds["range_sample"].attrs = {"long_name": "Along-range sample number, base 0"} | ||||||
| ds["echo_range"].attrs = {"long_name": "Range distance", "units": "m"} | ||||||
| if "range_sample" in ds: | ||||||
| ds["range_sample"].attrs = {"long_name": "Along-range sample number, base 0"} | ||||||
|
|
||||||
| if "echo_range" in ds: | ||||||
| ds["echo_range"].attrs = { | ||||||
| "long_name": "Range distance", | ||||||
| "units": "m", | ||||||
| } | ||||||
|
|
||||||
| if "frequency" in ds: | ||||||
| ds["frequency"].attrs = { | ||||||
| "long_name": "Frequency", | ||||||
| "units": "Hz", | ||||||
| } | ||||||
|
|
||||||
| ds[cal_type].attrs = { | ||||||
| "long_name": { | ||||||
| "Sv": "Volume backscattering strength (Sv re 1 m-1)", | ||||||
| "Sp": "Point scattering strength (Sp re 1 m^2)", | ||||||
| "TS": "Target strength (TS re 1 m^2)", | ||||||
| "Sv": "Volume backscattering strength (Sv re 1 m-1)", | ||||||
| "TS_spectrum": "Frequency-dependent target strength spectrum (TS(f) re 1 m^2)", | ||||||
| }[cal_type], | ||||||
| "units": "dB", | ||||||
| } | ||||||
|
Comment on lines
+237
to
260
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it'll be nice for us to have a consolidated place to specify all the attributes for variables. Right now we define these in an ad-hoc way and missed many of them. It seems would be a good idea to just have a YAML or similar to list the corresponding long_name, units, and other attributes for specific variables, and in functions that generates organized datasets, always pass the dataset through a function that calls the YAML to add attributes -- so like a decorator? I'll add an issue for this.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. tracked in #1747.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. one more take on this: |
||||||
|
|
@@ -348,6 +393,29 @@ def compute_Sv(echodata: EchoData, **kwargs) -> xr.Dataset: | |||||
| return _compute_cal(cal_type="Sv", echodata=echodata, **kwargs) | ||||||
|
|
||||||
|
|
||||||
| def compute_Sv_spectrum(echodata: EchoData, **kwargs) -> xr.Dataset: | ||||||
| """ | ||||||
| Compute frequency-dependent volume backscattering strength Sv(f) | ||||||
| from broadband EK80 complex data. | ||||||
|
|
||||||
| Notes | ||||||
| ----- | ||||||
| This functionality is not yet implemented. | ||||||
| """ | ||||||
| raise NotImplementedError("compute_Sv_spectrum is not yet implemented.") | ||||||
|
|
||||||
|
|
||||||
| def compute_Sp(echodata: EchoData, **kwargs) -> xr.Dataset: | ||||||
| """ | ||||||
| Compute point scattering strength (Sp) from raw data. | ||||||
|
|
||||||
| For CW data, Sp is computed from received power samples on the range grid. | ||||||
| For EK80 broadband/FM complex data, Sp is computed after pulse compression | ||||||
| and represents a band-averaged point-scattering-strength echogram. | ||||||
|
Comment on lines
+412
to
+414
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not exactly sure what you mean between "on the range grid" vs "point-scattering-strength echogram". Aren't both still on the range grid, either it's the original one or the one after pulse compression? |
||||||
| """ | ||||||
| return _compute_cal(cal_type="Sp", echodata=echodata, **kwargs) | ||||||
|
|
||||||
|
|
||||||
| def compute_TS(echodata: EchoData, **kwargs): | ||||||
| """ | ||||||
| Compute target strength (TS) from raw data. | ||||||
|
|
@@ -450,3 +518,48 @@ def compute_TS(echodata: EchoData, **kwargs): | |||||
| https://doi.org/10.1006/jmsc.2001.1158 | ||||||
| """ | ||||||
| return _compute_cal(cal_type="TS", echodata=echodata, **kwargs) | ||||||
|
|
||||||
|
|
||||||
| def compute_TS_spectrum(echodata: EchoData, **kwargs) -> xr.Dataset: | ||||||
| """ | ||||||
| Compute broadband frequency-dependent target strength spectrum, TS(f), | ||||||
| from EK80 broadband/FM complex data. | ||||||
|
|
||||||
| Parameters | ||||||
| ---------- | ||||||
| point_locations : xr.Dataset | ||||||
| Locations of targets for which TS(f) should be computed. | ||||||
| Must contain ``channel``, ``ping_time``, and ``target_range`` for each | ||||||
| ``target_id``. If ``target_range_min`` and ``target_range_max`` are | ||||||
| provided, they define the target echo segment. Otherwise, the segment | ||||||
| is built around ``target_range`` using ``NFFT`` and ``split_front``. | ||||||
|
|
||||||
| NFFT : int, optional | ||||||
| Number of FFT points used to compute the target spectrum. If not | ||||||
| provided, a value is inferred from the output frequency grid. | ||||||
|
|
||||||
| n_f_points : int, optional | ||||||
| Number of frequency points in the output TS(f) spectrum. Used when | ||||||
| ``frequency_resolution`` is not provided. | ||||||
|
|
||||||
| split_front : float, default 0.25 | ||||||
| Each echo spectrum is computed from a segment of the complex echo signal. | ||||||
| This parameter specifies how to position that segment around the target location | ||||||
| when only ``target_range`` is provided. For example, if ``split_front=0.25``, | ||||||
| then 25% of the NFFT window is placed before ``target_range`` and the remaining | ||||||
| 75% after it. | ||||||
|
|
||||||
| window : str, tuple, float or None, default None | ||||||
| Window passed directly to ``scipy.signal.get_window``. If ``None``, | ||||||
| a rectangular/boxcar window is used. | ||||||
|
|
||||||
| frequency_resolution : float, optional | ||||||
| Desired spacing of the output frequency grid in Hz. Used to define the | ||||||
| frequency grid on which TS(f) is evaluated. | ||||||
|
Comment on lines
+523
to
+558
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think allowing I also suggest |
||||||
|
|
||||||
| Returns | ||||||
| ------- | ||||||
| xr.Dataset | ||||||
| Dataset containing beam-compensated frequency-dependent target strength, TS(f). | ||||||
| """ | ||||||
| return _compute_cal(cal_type="TS_spectrum", echodata=echodata, **kwargs) | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This basically makes BB the default instead of FM. I think it'll be better to just wire everything to be based on FM right now, so that when we think it's time to remove support for the BB syntax, we just need to remove this line and the warning.