From 413d2c121afb56a8c149e1c003979f4a93f61b4b Mon Sep 17 00:00:00 2001 From: Hazen Babcock Date: Fri, 4 Sep 2026 09:28:28 -0400 Subject: [PATCH 1/6] Declare mFitRemoveRunningPeaks in multi_fit.h The function is defined and exported, and it is used: Python calls it through ctypes, which resolves the symbol from the shared library and so never consults a header. It was simply the one mFit* function missing from the declaration block. Nothing is broken today, but a C consumer wanting to call it would hit an implicit declaration warning under C99 and a hard error under C23, and the omission makes the header a misleading index of the library's API. Checked by comparing every mFit* definition in multi_fit.c against the declarations in multi_fit.h: with this added, the two agree. Co-Authored-By: Claude Opus 5 --- storm_analysis/sa_library/multi_fit.h | 1 + 1 file changed, 1 insertion(+) diff --git a/storm_analysis/sa_library/multi_fit.h b/storm_analysis/sa_library/multi_fit.h index 2c26fc91..48fa126b 100644 --- a/storm_analysis/sa_library/multi_fit.h +++ b/storm_analysis/sa_library/multi_fit.h @@ -181,6 +181,7 @@ double mFitPeakFgSum(fitData *, peakData *); double mFitPeakSum(fitData *, peakData *); void mFitRecenterPeaks(fitData *); void mFitRemoveErrorPeaks(fitData *); +void mFitRemoveRunningPeaks(fitData *); void mFitResetPeak(fitData *, int); void mFitSetPeakStatus(fitData *, int32_t *); int mFitSolve(double *, double *, int); From e9ffba5b42be1901d54ba657b88f37836b811ece Mon Sep 17 00:00:00 2001 From: Hazen Babcock Date: Fri, 4 Sep 2026 09:33:03 -0400 Subject: [PATCH 2/6] Drop the stale mFitIterateOriginal declaration multi_fit.h declared mFitIterateOriginal() but nothing defines it. The function was added in fa46abdc as an "(untested) iterate() function that duplicates the behavior of the original 3D-DAOSTORM approach", made to work in 93a99b7c, and removed by a19f27d4 on 2018-09-20, "Use circular ROI for fitting", which left the declaration behind. Its parent commit still has the definition. Nothing in the tree has referenced it since, in C or through ctypes. This is the same shape as the mFitPeakFgSumSensitivityCorrected declaration removed in 0847a07d, and the other half of what makes the header a poor index of the library: 413d2c12 added the one declaration that was missing, this removes the one that outlived its function. With both, every mFit* definition in multi_fit.c has a declaration in multi_fit.h and every declaration has a definition, 38 each way. Co-Authored-By: Claude Opus 5 --- storm_analysis/sa_library/multi_fit.h | 1 - 1 file changed, 1 deletion(-) diff --git a/storm_analysis/sa_library/multi_fit.h b/storm_analysis/sa_library/multi_fit.h index 48fa126b..e920ad2f 100644 --- a/storm_analysis/sa_library/multi_fit.h +++ b/storm_analysis/sa_library/multi_fit.h @@ -171,7 +171,6 @@ void mFitGetResidual(fitData *, double *); int mFitGetUnconverged(fitData *); fitData *mFitInitialize(double *, double *, double, int, int); void mFitInitializeROIIndexing(fitData *, int); -void mFitIterateOriginal(fitData *); void mFitIterateLM(fitData *); void mFitNewBackground(fitData *, double *); void mFitNewImage(fitData *, double *); From cb24ec83bd8f656edf5d1957b9666f557d1d2a14 Mon Sep 17 00:00:00 2001 From: Hazen Babcock Date: Fri, 4 Sep 2026 09:36:44 -0400 Subject: [PATCH 3/6] Give HYSTERESIS its real value and use it HYSTERESIS was defined and never referenced, while mFitUpdate() decided when to recentre an AOI from two literals: if((dx<-0.5)||(dx>1.5)){ Reading the constant the way its comment does, as a threshold on how far a peak may drift from its AOI before the AOI follows, |dx - 0.5| > HYSTERESIS: 0.5 dx < 0.0 or dx > 1.0 band [0, 1] no hysteresis 0.6 dx < -0.1 or dx > 1.1 band [-0.1, 1.1] 1.0 dx < -0.5 or dx > 1.5 band [-0.5, 1.5] what the code does So the constant was not documenting a dropped feature. Hysteresis is live, there is exactly one implementation of it, and every other fitter reaches it by calling mFitUpdate(). The constant simply carried the wrong number, 0.6 against an effective 1.0, and nothing tied the two together. Set it to 1.0 and write the bounds in terms of it. Taking 0.6 as authoritative instead would narrow the dead band to [-0.1, 1.1], which is a real change to peak recentring during fitting and does not belong in a branch about dead code. Behaviour preserving, and provably so: multi_fit.o is byte identical before and after, md5 4b8058df50f0d1b692237950820df4aa. gcc folds 0.5-1.0 and 0.5+1.0 to the same literals the code had. Co-Authored-By: Claude Opus 5 --- storm_analysis/sa_library/multi_fit.c | 4 ++-- storm_analysis/sa_library/multi_fit.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/storm_analysis/sa_library/multi_fit.c b/storm_analysis/sa_library/multi_fit.c index 54c94be5..ca81f46c 100644 --- a/storm_analysis/sa_library/multi_fit.c +++ b/storm_analysis/sa_library/multi_fit.c @@ -1929,12 +1929,12 @@ void mFitUpdate(peakData *peak) double dx,dy; dx = peak->params[XCENTER] - peak->xi; - if((dx<-0.5)||(dx>1.5)){ + if((dx<(0.5-HYSTERESIS))||(dx>(0.5+HYSTERESIS))){ peak->xi = (int)floor(peak->params[XCENTER]); } dy = peak->params[YCENTER] - peak->yi; - if((dy<-0.5)||(dy>1.5)){ + if((dy<(0.5-HYSTERESIS))||(dy>(0.5+HYSTERESIS))){ peak->yi = (int)floor(peak->params[YCENTER]); } } diff --git a/storm_analysis/sa_library/multi_fit.h b/storm_analysis/sa_library/multi_fit.h index e920ad2f..4559fa91 100644 --- a/storm_analysis/sa_library/multi_fit.h +++ b/storm_analysis/sa_library/multi_fit.h @@ -34,7 +34,7 @@ #define CONVERGED 1 #define ERROR 2 -#define HYSTERESIS 0.6 /* In order to move the AOI or change it's size, +#define HYSTERESIS 1.0 /* In order to move the AOI or change it's size, the new value must differ from the old value by at least this much (<= 0.5 is no hysteresis). */ From 863f51bcdfac497c8a98ddb6f82adecee6f02acb Mon Sep 17 00:00:00 2001 From: Hazen Babcock Date: Fri, 4 Sep 2026 09:39:27 -0400 Subject: [PATCH 4/6] Guard dxfAt3D's range checks with RANGECHECK Its six siblings, dxfAt2D, dyfAt2D, dyfAt3D, dzfAt3D, fAt2D and fAt3D, all wrap their rangeCheckI() calls in if(RANGECHECK). dxfAt3D called them unconditionally. No effect while RANGECHECK is 1: cubic_spline.o is byte identical before and after, md5 b0b37b59a7cd4270e7014183ed032699. The point is what happens when it is turned off for speed, which is the only reason the macro exists. Compiling both versions with RANGECHECK 0: dxfAt3D instructions "dxfAt3D,xc" etc in .o size unguarded 46 3 9608 guarded 17 0 9144 So the unguarded version keeps paying for all three checks, and keeps their message strings, in exactly the build where the user asked not to. Note that gcc inlines rangeCheckI() at -O3, so there is no call instruction to look for, which is why this is measured by instruction count and string table rather than by disassembly search. Co-Authored-By: Claude Opus 5 --- storm_analysis/spliner/cubic_spline.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/storm_analysis/spliner/cubic_spline.c b/storm_analysis/spliner/cubic_spline.c index e867fffd..60962d52 100644 --- a/storm_analysis/spliner/cubic_spline.c +++ b/storm_analysis/spliner/cubic_spline.c @@ -206,9 +206,11 @@ double dxfAt3D(splineData *spline_data, int zc, int yc, int xc) { double yv; - xc = rangeCheckI("dxfAt3D,xc", xc, 0, spline_data->xsize); - yc = rangeCheckI("dxfAt3D,yc", yc, 0, spline_data->ysize); - zc = rangeCheckI("dxfAt3D,zc", zc, 0, spline_data->zsize); + if(RANGECHECK){ + xc = rangeCheckI("dxfAt3D,xc", xc, 0, spline_data->xsize); + yc = rangeCheckI("dxfAt3D,yc", yc, 0, spline_data->ysize); + zc = rangeCheckI("dxfAt3D,zc", zc, 0, spline_data->zsize); + } yv = dot(&(spline_data->aij[(xc*(spline_data->ysize*spline_data->zsize)+yc*spline_data->zsize+zc)*64]), spline_data->delta_dxf, 64); From ff68de75cd0c8c817317d7ce6e001df2c701b1d4 Mon Sep 17 00:00:00 2001 From: Hazen Babcock Date: Fri, 4 Sep 2026 09:47:50 -0400 Subject: [PATCH 5/6] Collapse the identical if/else in daoInitialize The two branches were byte identical: if((roi_size%2)==0){ fit_data->xoff = 0.5*((double)roi_size) - 1.0; fit_data->yoff = 0.5*((double)roi_size) - 1.0; } else{ fit_data->xoff = 0.5*((double)roi_size) - 1.0; fit_data->yoff = 0.5*((double)roi_size) - 1.0; } The comment above them explains why, and it is worth keeping: the author tested even and odd ROI sizes and found the same offset optimal for both. But a branch that does not branch reads as an unfinished edit, and a reader has to compare the two bodies character by character to find out that it is deliberate. Say it in the comment instead, which is where it belongs. Behaviour preserving, and provably so: dao_fit.o is byte identical before and after, md5 b2dc5b13076ae6d61574c86d6345c5ad. gcc was already collapsing this. This is also what makes 3acb33f0 harmless, since both branches assign xoff and yoff the same expression. Co-Authored-By: Claude Opus 5 --- storm_analysis/sa_library/dao_fit.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/storm_analysis/sa_library/dao_fit.c b/storm_analysis/sa_library/dao_fit.c index 6c671d2b..220ca93a 100644 --- a/storm_analysis/sa_library/dao_fit.c +++ b/storm_analysis/sa_library/dao_fit.c @@ -1265,17 +1265,12 @@ fitData* daoInitialize(double *rqe, double *scmos_calibration, double tol, int i mFitInitializeROIIndexing(fit_data, roi_size); /* - * I thought that even/odd might need different offsets, but based on some testing - * it looks like the optimal offset is pretty similar / the same for both. + * I thought that even/odd ROI sizes might need different offsets, but based + * on some testing it looks like the optimal offset is pretty similar / the + * same for both, so roi_size%2 is deliberately not consulted here. */ - if((roi_size%2)==0){ - fit_data->xoff = 0.5*((double)roi_size) - 1.0; - fit_data->yoff = 0.5*((double)roi_size) - 1.0; - } - else{ - fit_data->xoff = 0.5*((double)roi_size) - 1.0; - fit_data->yoff = 0.5*((double)roi_size) - 1.0; - } + fit_data->xoff = 0.5*((double)roi_size) - 1.0; + fit_data->yoff = 0.5*((double)roi_size) - 1.0; fit_data->fit_model = (daoFit *)malloc(sizeof(daoFit)); dao_fit = (daoFit *)fit_data->fit_model; From 48fcac6c4b17a44a3f9979974c86c92a3c8e710a Mon Sep 17 00:00:00 2001 From: Hazen Babcock Date: Fri, 4 Sep 2026 09:52:20 -0400 Subject: [PATCH 6/6] Remove the dead daoInitializeZ argtypes assignment loadDaoFitC() set daoInitializeZ.argtypes twice, two lines apart: daofit.daoInitializeZ.argtypes = [ctypes.c_void_p] daofit.daoInitializeZ.argtypes = [ctypes.c_void_p, ndpointer(dtype=numpy.float64), ndpointer(dtype=numpy.float64), ctypes.c_double, ctypes.c_double] The first is overwritten immediately and never has any effect. The second is the correct one: it matches dao_fit.h:75, void daoInitializeZ(fitData *, double *, double *, double, double); and the call site in MultiFitterZ.initializeC(), which passes five arguments. Had the one argument version been the one that survived, ctypes would have rejected every call. Checked that this was the only such case. A scan of every *_c.py wrapper for a ctypes argtypes or restype assigned more than once in the same module finds nothing else; run against this file before the fix it does report daofit.daoInitializeZ.argtypes, so it is capable of finding them. Co-Authored-By: Claude Opus 5 --- storm_analysis/sa_library/dao_fit_c.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/storm_analysis/sa_library/dao_fit_c.py b/storm_analysis/sa_library/dao_fit_c.py index df289922..93269cf8 100644 --- a/storm_analysis/sa_library/dao_fit_c.py +++ b/storm_analysis/sa_library/dao_fit_c.py @@ -222,8 +222,6 @@ def loadDaoFitC(): ctypes.c_double, ctypes.c_double] - daofit.daoInitializeZ.argtypes = [ctypes.c_void_p] - daofit.daoInitializeZ.argtypes = [ctypes.c_void_p, ndpointer(dtype=numpy.float64), ndpointer(dtype=numpy.float64),