socolofs / tamoc

Texas A&M Oilspill Calculator
MIT License
34 stars 13 forks source link

Errors in ambient.compute_pressure? #27

Open nordam opened 1 year ago

nordam commented 1 year ago

The function ambient.compute_pressure seems to have a mistake in the example in the docstring, and a bug. It returns negative pressure values when I call it with the example values given in the docstring:

z = np.arange(-1500, 0, 10)
T = np.linspace(4.1, 25.0, len(z)+1)
S = np.linspace(36.5, 34.5, len(z)+1)
fs_loc = -1
P = ambient.compute_pressure(z, T, S, fs_loc)

>>> z[-1]
-10
>>> P[-1]
-1356071.446785233
>>> z[0]
-1500
>>> P[0]
-182884771.0828263

According to the docstring description of T, it should be in Kelvin, and if I convert T to Kelvin by adding 273, it returns more reasonable values:

>>> P[-1]
202273.1132283202
>>> P[0]
15253980.602979604

Furthermore, the argument fs_loc doesn't appear to work as intended. If I reverse the order of the z array, and change fs_loc from -1 to 0, I get the following error:

IndexError: index 150 is out of bounds for axis 0 with size 150
ChrisBarker-NOAA commented 1 year ago

Thanks Tor.

As it happens, I was just looking at this (oddly hadn't noticed the docstring error). But I did write a unit test. It seems we need another one that tests the fs_loc flag.

You can find it in #24, and in the (now miss-named) numpy_updates branch.

tamoc/test/test_ambient.py

If you add another test (even, or especially) if it fails, that would be great.

NOTE: I also added code to ambient.py that would convert to K before generating a pressure profile -- I got bitten by the same issue.