Added `depth_from_pressure` method required for the calculation of `vertical_offset` value

[praneethratna]

Partial fix for 2nd step of https://github.com/OSOceanAcoustics/echopype/issues/1181#issuecomment-1763217291, by including calculations for depth value as mentioned in https://github.com/OSOceanAcoustics/echopype/issues/1181#issuecomment-1781696782.

[codecov-commenter]

Codecov Report

Merging #1207 (07bf9f4) into dev (9dfe5ba) will increase coverage by 4.14%. Report is 1 commits behind head on dev. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##              dev    #1207      +/-   ##
==========================================
+ Coverage   77.08%   81.22%   +4.14%     
==========================================
  Files          67        7      -60     
  Lines        5921      522    -5399     
==========================================
- Hits         4564      424    -4140     
+ Misses       1357       98    -1259     

Flag	Coverage Δ
unittests	81.22% <100.00%> (+4.14%)	:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
echopype/utils/misc.py	100.00% <100.00%> (ø)

... and 64 files with indirect coverage changes

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[emiliom]

Thanks @praneethratna ! I'm working on this.

[emiliom]

See the PR I submitted against your PR, with enhancements. This will be on your echopype fork.

[emiliom]

@praneethratna I'm moving your comment to the main PR, here, since it's a broader comment and not specific to my edits to your PR (I know, it's confusing!):

I just now remember that in the _compute_pressure method we subtract 10.125 from pressure value everytime. Should we remove this atm_pres_surf argument then?

For reference, here's the Matlab function you're referring to: LoadAZFP.m, computeDepth.

And here's the statement in depth_from_pressure that subtracts atm_pres_surf from pressure:

pressure = pressure - atm_pres_surf

[emiliom]

I don't think we should remove the atm_pres_surf argument. The broader question is whether pressure measurements that a user may pass to depth_from_pressure would ever have atmospheric pressure at the surface already removed. If that's the case, then the function should not apply a correction for a constant atmospheric pressure value.

That's why we're including atm_pres_surf as a function argument, but also why it's optional and has a default value. I think the main question is what default value would be most useful: 0, or 10.1325 dbar. Right now it's 0. I'll ask around to come to a decision, but let's discuss that here, not in my "PR to your PR", since I didn't change the atm_pres_surf value in that PR.

For reference: The CF convention specifies two different sea water pressure standard names, depending on whether the overlying pressure has been excluded:

• https://mmisw.org/ont/cf/parameter/sea_water_pressure
• https://mmisw.org/ont/cf/parameter/sea_water_pressure_due_to_sea_water

[praneethratna]

I don't think we should remove the atm_pres_surf argument. The broader question is whether pressure measurements that a user may pass to depth_from_pressure would ever have atmospheric pressure at the surface already removed. If that's the case, then the function should not apply a correction for a constant atmospheric pressure value.

I was actually thinking only for use case of setting vertical_offset. I missed the point that the function can be used by a user independently as well.

That's why we're including atm_pres_surf as a function argument, but also why it's optional and has a default value. I think the main question is what default value would be most useful: 0, or 10.1325 dbar. Right now it's 0. I'll ask around to come to a decision, but let's discuss that here, not in my "PR to your PR", since I didn't change the atm_pres_surf value in that PR.

I think it should be 10.1325 dbar? Since we take pressure in dbar.

[emiliom]

I've confirmed that pressure sensors can be set (calibrated) to return a value of 0 at the surface. That's probably not an unusual configuration. So, let's keep the default atm_pres_surf value at 0. I will make an inline suggestion with edits to the doc string to make this clearer. Then, we can merge this PR!