Fix Specific volume second derivative SA P mistake (from Matlab)

[ocefpaf]

Port TEOS-10/GSW-Matlab@38c9635

Thanks @richardsc for pointing this out.

Closes #52

---

We need to update the test values for:

specvol_second_derivatives (v_sa_p)........................... << failed >>
rho_second_derivatives (rho_sa_p)............................. << failed >>
thermobaric................................................... << failed >>

[ocefpaf]

If I update to the latest matlab file we fix those but get more errors, probably other updates that got dumped in the matlab code.

c_from_sp..................................................... << failed >>
sp_from_c..................................................... << failed >>
geo_strf_dyn_height........................................... << failed >>
specvol_second_derivatives_wrt_enthalpy (v_sa_sa_wrt_h)....... << failed >>
rho_second_derivatives_wrt_enthalpy (rho_sa_sa_wrt_h)......... << failed >>

[ocefpaf]

Commit d8c48ed fixed these:

specvol_second_derivatives_wrt_enthalpy (v_sa_sa_wrt_h)....... << failed >>
rho_second_derivatives_wrt_enthalpy (rho_sa_sa_wrt_h)......... << failed >>
geo_strf_dyn_height........................................... << failed >>

3 more to go!

[castelao]

Good catch on d8c48ed

[efiring]

geo_strf_dyn_height is expected to fail with the matlab check values; we are using a different algorithm, so we need to patch in our own check values. See #36.

[efiring]

Are the data and check values in this PR now coming from gsw_matlab_v3_06_16.zip?

[ocefpaf]

Are the data and check values in this PR now coming from gsw_matlab_v3_06_16.zip?

Yep. Downloaded from https://www.teos-10.org/software/gsw_matlab_v3_06_16.zip and sent as a PR to TEOS-10/GSW-Matlab#15 for tracking purposes.

geo_strf_dyn_height is expected to fail with the matlab check values; we are using a different algorithm, so we need to patch in our own check values. See #36.

I thought that the check data

GSW-C/make_data_from_mat.py

Lines 334 to 336 in b5ecb2a

	if name == "geo_strf_dyn_height":
	geo_strf = get_strf_dyn_height_from_npy()
	cv[name] = geo_strf

patched it. I use that script to create the new data.

[efiring]

You are correct, running make_data_from_mat.py should take care of the patch, provided none of the inputs to the test have changed.
I also did a quick check to compare the constants and code in sp_from_c between our version and the matlab, and I don't see anything that could account for the difference. This might take some deep digging, starting with isolating a single set of input check values out of the whole array being tested.

[efiring]

I'm making some progress in tracking it down; it looks like a change in the check values for conductivity in the fresh-water case; the failure now is occurring in a value that was masked before.

[PaulMBarker]

I do not remember changing the code in SP_from_C or C_from _SP.
The check values should not have changed unless I have accidently entered something.

[castelao]

@PaulMBarker , just to clarify. The check values should be the same, but would those be still adequate to validate SP_from_C and C_from_SP after you changed the code?

It's intriguing. I'm using the data from 3_06_16 to validate those functions in Rust with success. It is interesting that in other functions I can match the validation dataset with precision epsilon, but in this group of functions the difference was slightly larger.

[PaulMBarker]

If your code matches my matlab code then the values will be correct.
Did you find a change? if so what was it?

[efiring]

(Edited, 2022-10-07, to correct the description of the cast where the failures occur.)

@PaulMBarker The comparisons here are between what was in the gsw_matlab_v3_06_11 zipfile and the gsw_matlab_v3_06_16 zipfile.
The gsw_C_from_SP.m code has not changed (apart from the comment giving the date).
The values in the *chck_cast arrays (with 3 profiles each) have changed by fairly small amounts; I have no idea why.
Correspondingly, the check values have also changed by similarly small amounts.

The failures in the two tests in the C code, for C_from_SP and SP_from_C are not related to the
ubiquitous small changes described above. Instead, they arise from a different change in the check values for
C_from_SP, specifically in the 3rd column, the short "cast" in relatively fresh water (SP < 11). In the _11 version, the first 6 of
those check values are nan (SP < 9 for those), and are followed by 2 non-nan values (SP is approx 9.06 and 10.3); in the _16 version, all 8 low-SP
check values are non-nan. So, previously in the C code, only the last 2 of the low-SP calculations of C
were being tested, but now all 8 of them are, and some of those 8 are failing.

I have not yet looked closely at the low-S part of the C code in C_from_SP to see whether it is somehow different from
the matlab version, and I have not determined how many of the 8 values are failing. I know the first one (for P=0) is
failing, and has the maximum error (which is still very small, but well beyond the given tolerance).

Once C_from_SP has failed, it is inevitable that SP_from_C will also fail, since the latter is using the output of the former.

[castelao]

@efiring and @PaulMBarker , thanks for the info.

[efiring]

I have found the problem: a difference in the sign of coefficient q7.
@ocefpaf, can I push to this branch on your repo? I also need to fix the check values for geo_strf_dyn_height,
which is most easily done by putting all the C updates and fixups in GSW-Python first. I also have a small change
to the C gsw_check executable that makes debugging a little easier when problems like this arise.

[ocefpaf]

I have found the problem: a difference in the sign of coefficient q7.
@ocefpaf, can I push to this branch on your repo?

Done!

I also need to fix the check values for geo_strf_dyn_height,
which is most easily done by putting all the C updates and fixups in GSW-Python first. I also have a small change
to the C gsw_check executable that makes debugging a little easier when problems like this arise.

If you don't mind let's merge this one as-is then, with only the geo_strf_dyn_height failure now, and fix the rest on a new PR.

---

geo_strf_dyn_height........................................... << failed >>

  Max difference = 0.0012001065745366191, limit = 5.5249752506369987e-07
  Max diff (rel) = 0.0037709490696147497, limit = 1.7360458415185846e-06
geo_strf_dyn_height_pc........................................... passed ( 98)
geo_strf_dyn_height_pc (geo_strf_dyn_height_pc_p_mid)............ passed ( 98)