search

CIRDLES / Squid

Squid3 is being developed by the Cyber Infrastructure Research and Development Lab for the Earth Sciences (CIRDLES.org) at the College of Charleston, Charleston, SC and Geoscience Australia as a re-implementation in Java of Ken Ludwig's Squid 2.5. - please contribute your expertise!
http://cirdles.org/projects/squid/
Apache License 2.0
12 stars 27 forks source link

Spot-means via Linear Regression in Squid3 appear to be Intercepts (t = 0), not burn-midpoints #728

Closed sbodorkos closed 1 year ago

sbodorkos commented 1 year ago

Thanks to Bill Davis (Geological Survey of Canada) for flagging this. He has processed data in both Squid3 and SQUID 2.50.20.10.26 (which is the "best match" to Squid3), and noted that spot-means are indistinguishable when Squid3 is used in 'spot-average' mode ("Wtd Mean" below), but quite different in 'linear regression' mode ("LR" below). In particular, the Squid3 LR uncertainties are significantly larger than the SQUID 2.50 LR uncertainties:

image

Bill suggested that it might be a manifestation of this problem (this is a slide from my Squid3 Launch PowerPoint):

image

I have reviewed both sets of code, and I think he might be right. At some point, the "midpoint vs intercept" issue was addressed in SQUID 2.50: the 26 October 2020 version contains the amended code:

IfLinReg

which calculates Slope, SigmaSlope and CovSlopeInter whenever LinReg = TRUE (regardless of MinIndex).

In contrast, the corresponding code documentation for Squid3 (see https://github.com/CIRDLES/ET_Redux/wiki/SHRIMP:-Sub-WtdLinCorr, right at the bottom) looks like this:

image

It appears that the MinIndex-related criterion was flagged for follow-up, but this line of code was never changed in Squid3. My guess is we never picked this up previously because we have never done a cell-by-cell comparison (for data calculated via linear regression) since the SQUID 2.50 change was enacted.

Jim @bowring , could you please review the Squid3 code corresponding to the yellow line, and modify if needed? It should simply read "If LinReg = TRUE". When we have a new version, I will ask Bill to re-test.

If that line of code is not the issue (i.e. if it already just reads "If LinReg = TRUE"), then we will need to dig deeper. Please let me know.

bowring commented 1 year ago

Found the code with the minIndex condition at line 198 of WeightedMeanCalculators.java.

        if (linReg && (minIndex > 0)) {
               wtdLinCorrResults.setSlope(slopeW[minIndex]);
               wtdLinCorrResults.setSigmaSlope(sigmaSlopeW[minIndex]);
               wtdLinCorrResults.setCovSlopeInter(covSlopeInterW[minIndex]);
        }

Will update and release a new version this week. Thanks for checking the details!