Fix NormalizedCrossCorrelationMean to centre by the matched mean (match its docstring)#28
Open
jkosinski wants to merge 5 commits into
Open
Fix NormalizedCrossCorrelationMean to centre by the matched mean (match its docstring)#28jkosinski wants to merge 5 commits into
jkosinski wants to merge 5 commits into
Conversation
…he global array mean The docstring and the score name define the Pearson correlation about the mean over the matched points: subtract mean(target_weights) and mean(template_weights). The implementation instead subtracted the *global* target-array mean in __init__ (target - target.mean()) — a constant offset that does not centre the overlapping values, so the score did not match its own documentation. When the template covers a small high-signal region of a largely empty map (a subunit, or any contour/envelope-restricted point cloud) the global mean ≈ 0 while the matched mean is large, so the results differ substantially. Fix: compute the correlation in __call__ by centring the matched target values (self._target_values) and the template weights by their own means — the documented Pearson r over the overlap. It now equals Chimera "measure correlation ... envelope true" (the cam reported by efitter/Assembline) when given a contour-thresholded template point cloud. Note: the inherited analytic gradient is not updated for the mean-subtracted form; use a gradient-free optimizer (optimize_match's default basinhopping).
Update the inherited gradient for the mean-subtracted form. NormalizedCrossCorrelationMean inherited NormalizedCrossCorrelation.grad (the gradient of the NON-centred score), inconsistent with the mean-subtracted __call__. Add NormalizedCrossCorrelationMean.grad = the same formula on the centred values v-mean(v), w-mean(w); the mean-subtraction adds no terms because sum(v-mean(v)) = sum(w-mean(w)) = 0. Verified numerically: cosine +0.999 with finite differences and the same scale factor as NormalizedCrossCorrelation.grad. (cherry picked from commit 6cf65790df5c844313b0d1d72a282eedd43825ee)
… (review) - grad() returns score_sign * total_grad, so it is correct for both negate_score settings (the parent CrossCorrelation/NormalizedCrossCorrelation grads hardcode -total_grad, valid only for negate_score=True). - Replace non-ASCII symbols (combining diacritics, em dash) in the comment/docstring with ASCII to avoid the bidirectional-Unicode source warning. - Add TestNormalizedCrossCorrelationMean: score equals the matched Pearson correlation; differs from the old global-mean centring under a strong local offset; finite-difference direction of grad(); and grad sign follows negate_score. (cherry picked from commit 21d49db9b2266f5b063a5fd55a89aebdffca244c)
Author
|
Out of scope for this PR, noted for later: |
…cription) (cherry picked from commit 8f3136070cb6796c78beae09880283725207c7b4)
Use be.mean / be.dot / be.sqrt(be.sum(be.square(...))) instead of numpy in NormalizedCrossCorrelationMean.__call__ and grad, so they run on any backend (numpy/cupy/jax), consistent with the rest of matching_optimization. Numerically identical (tests unchanged). (cherry picked from commit 4cf16848b68bfbdcdb705618439b3f4c9ad55118)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
NormalizedCrossCorrelationMeannow centres by the mean of the matched target values (the mean of the target over the current template footprint), so it computes the documented Pearson-about-the-mean -- the formula in its class docstring and implied by the...Meanname.__call__previously subtracted the global target-array mean, a constant offset that does not centre the overlapping values, so the score was not the documented quantity. This changesNormalizedCrossCorrelationMean's output whenever the matched footprint's local mean differs from the whole-array mean (i.e. essentially always).Gradient
Adds
NormalizedCrossCorrelationMean.grad. Previously it inheritedNormalizedCrossCorrelation.grad-- the gradient of the non-centred score -- which is inconsistent with the mean-subtracted__call__. The new gradient has the same form asNormalizedCrossCorrelation.gradevaluated on the centred valuesv - mean(v),w - mean(w); the mean-subtraction adds no extra terms becausesum(v - mean(v)) = sum(w - mean(w)) = 0.It returns
score_sign * d(score)/dp, so it is correct for bothnegate_scoresettings. Verified numerically: cosine +0.999 with finite differences and the same scale factor asNormalizedCrossCorrelation.grad(both use the Sobel-estimated spatial derivative). The defaultoptimize_match(gradient-free basinhopping) is unaffected; this only enables correct gradient-based optimization for the score.Tests
TestNormalizedCrossCorrelationMean:grad();grad()sign followsnegate_score.