C computed values not at expected magnitude

[asx-]

cond_dict() puts out a value one order of magnitude higher than expected for S/m, requiring a two order of magnitude correction for mS/cm. Manually typed values from a calibration sheet work as expected. Possible bug somewhere in libODF_sbe_reader.py or libODF_convert.py making the values an order of magnitude off, it should be corrected to clean up cond_dict() cases.

[webbpinner]

I think I see the problem...

we're defining the units as 'S/m'. (libODF_convert.py line 14)

now look at lines 190-191 in libODF_sbe_equations_dict.py: if units == 'mS': temp = temp * 0.01

I think we need to change 'mS' to 'S/m' in libODF_sbe_equations_dict.py or change 'S/m' to 'mS' in libODF_convert.py

[webbpinner]

just checking… did you see my comments to the 2 bugs you posted?

-W

=-------------------------------------------------= Webb Pinner Mobile: 401.749.9322 Email: webbpinner@gmail.com Website: http://www.oceandatarat.org

On Nov 30, 2016, at 3:07 PM, asx- notifications@github.com wrote:

cond_dict() puts out a value one order of magnitude higher than expected for S/m, requiring a two order of magnitude correction for mS/cm. Manually typed values from a calibration sheet work as expected. Possible bug somewhere in libODF_sbe_reader.py or libODF_convert.py making the values an order of magnitude off, it should be corrected to clean up cond_dict() cases.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/asx-/odf-ctd-proc/issues/10, or mute the thread https://github.com/notifications/unsubscribe-auth/ABfvVASUswsY_1q7ULkbZO_v9P0e36Jkks5rDdd9gaJpZM4LAn6Z.

[asx-]

I just added those lines in as a shim, if you use the S/m mode (which multiplies by 0.1) it'll put out the expected values for S/m from a cast file, but multiplying by 0.1 causes calibration data points to be off by an order of magnitude. There's a bug somewhere that is throwing off the magnitude, maybe in _parse_scans(), but the magnitude is different from the precision, where I was able to change the range of precision simply by rounding off values here and there in cond_dict(). I suspect SBE may be doing something funny on their side after spending half a day yesterday looking at our implementation and testing it, but I haven't proved it conclusively yet.

libODF_convert should default to mS/cm, our analysis code all works in mS/cm instead of S/m.

Because the magnitude change from S/m to mS/cm effectively masks the error right now and we'll do reprocessing before final datasets, they should both be marked as a low priority until the cruise is finished

[webbpinner]

Trying to wrap my head around the math... Can you verify this units math (little rusty when it comes to units) S/m = 100S/cm = (100*.001)mS/cm = 0.1mS/cm

What units should the sensor be providing?

What length is implied in libODF_sbe_equations_dict.py

Thanks, -W

[asx-]

0.1mS/cm == 1 S/m unless my math is wrong too.

Our analysis software works in mS/cm because it works out better If a reading should be 2.45673 S/m (made up) The sensor might report 2.45675 S/m The current conversion to mS/cm would output 0.24567

Take a look at test_equations.py, it has hardcoded values from a calib file that should describe what's going on, although you need to alter the magnitude factor in libODF_sbe_equations_dict.py. The first non-zero value for cond is some equation solving to try to fix the computational error.

[asx-]

Slightly OOT: conductivity seems to be the only one that has bugs in it right now. Temperature checked out fine, pressure seems to be fine after fixing the temperature source but needs to be verified against multiple pressure calibration documents if possible, and oxygen seems to be fine. A long way down the line is to get every calibration doc we can get our hands on and input them as unit tests, which could be neatly tied in farther down the line once we get to CTD acquisition.

[webbpinner]

Do we know what the actual units the sensor measures? S/m, S/cm, mS/m, etc??

It’s easy enough to expand the if statements to accommodate multiple conversions depending on the starting units.

=-------------------------------------------------= Webb Pinner Mobile: 401.749.9322 Email: webbpinner@gmail.com Website: http://www.oceandatarat.org

On Nov 30, 2016, at 5:44 PM, asx- notifications@github.com wrote:

Slightly OOT: conductivity seems to be the only one that has bugs in it right now. Temperature checked out fine, pressure seems to be fine after fixing the temperature source but needs to be verified against multiple pressure calibration documents if possible, and oxygen seems to be fine. A long way down the line is to get every calibration doc we can get our hands on and input them as unit tests, which could be neatly tied in farther down the line once we get to CTD acquisition.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/asx-/odf-ctd-proc/issues/10#issuecomment-264020686, or mute the thread https://github.com/notifications/unsubscribe-auth/ABfvVIQZhOEzlJAHgxxVI85JSnIRwoHbks5rDfxdgaJpZM4LAn6Z.

[asx-]

Engineering units is Hertz, their calibration sheets are in S/m

[webbpinner]

so the code actually makes sense.

if your raw values are mS/m (designated by units = 'mS') then dividing by 100 temp = temp .01 would get you to mS/cm. we're trying to say that the raw values are S/m (designated by units = 'S') thus we need to say temp = temp .1. The code actually includes this logic. Here's the full loop:

    Conductivity = []
    f = [x/1000 for x in F]
    for F_0, t_0, p_0 in zip(f, t, p):
        temp = ((calib['G'] + calib['H'] * math.pow(F_0,2)
                 + calib['I'] * math.pow(F_0,3)
                 + calib['J'] * math.pow(F_0,4))
                / (1 + calib['CTcor'] * t_0 + calib['CPcor'] * p_0))
        #S/m to mS/cm
        if units == 'mS':
            temp = temp * 0.01
        elif units == 'S':
            temp = temp * 0.1
        temp = round(temp, 5)
    Conductivity.append(temp)

Here's the function definition: cond_dict(calib, F, t, p, units='mS')

Here's how it's being called in libODF_convert: sbe_eq.cond_dict(temp_meta['sensor_info'], raw_df[temp_meta['column']], t_array, p_array)

Notice the last 'units' argument isn't specified in the call so it defaults to 'mS' which cause the loop logic to multiply the output by .01. If we define the units to 'S' (which matches the lookup table, we just made it say S/m) then the sbe_eq.cond_dict mulitplies by only .1.

I haven't had a chance to play with the testing file which I should probably do before I make a complete fool of myself.

[asx-]

Yes, the code is fully functional and is 99% working as intended right now. There are two things I want to fix: -move precision from 4 decimal places to 5 decimal places when computing S/m, thereby matching SBE calibration docs/SBE format exactly (the last 1%) -figure out where the extra order of magnitude comes in and remove it to simplify the code

[asx-]

FYI cond_dict() has been updated re: magnitude and mS/cm seems to work correctly. From:

#S/m to mS/cm
if units == 'mS':
    temp = temp * 0.01

To:

#S/m to mS/cm
if units == 'mS':
    temp = temp * 1

This causes our analysis to look correct, namely all other variables dependent on conductivity to appear correct when graphed. However, when switched for mS/cm the calibration reference values output the correct values for S/m, so there is still a order of magnitude off that should be tracked down in the future. The magnitude change up also increases the error reported in values, which makes it more important to look for the discrepancy in reporting.

Also: 1000mS / 100cm = 10, instead of (1/1000)mS / (1/100)cm = 0.1, which should help us with trying to figure out why it all seems so wrong in the future

I'm going to leave this open for now because the engineering units and the equation leads me to believe the correct output by default is S/m, not mS/cm, so there's still something funky in there. But it's definitely low priority for now.