Stabilize Harmony optimizer updates - #293
Open
fderop wants to merge 1 commit into
Open
Conversation
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.
Problem
Harmony can report convergence after the outer objective increases. Direct float exponentiation can also produce nonfinite assignments and objectives for valid parameter values.
A run can then return finite corrected coordinates with invalid optimizer state. Results will change for affected runs that stopped early or produced nonfinite assignments.
Original code
The assignment code exponentiates unshifted float scores. If all cluster weights underflow for one cell, normalization divides by zero.
An objective increase makes the relative decrease negative. Every negative value is less than a positive epsilon, so the code reports convergence.
Evidence
The bundled
cell_lines_smalldata contains 300 cells. A normal one-variable run useddataset,theta=1,sigma=0.02, and five clusters.The old code stopped after three outer rounds when the objective increased:
A float build also failed with donor labels,
theta=1,sigma=0.01, and five clusters. Assignments and objectives became nonfinite, but corrected coordinates stayed finite.Fix
Assignment scores now use double-precision log space. The code adds diversity terms before it subtracts each cell maximum and normalizes the scores.
Outer convergence now requires a finite, nonnegative relative decrease. A nonfinite optimizer state now raises a numerical error before correction or return.
The log-space assignment update contains the same multi-covariate product as #292.
Verification
The same dataset run now completes all ten requested outer rounds. It does not label the objective increase as convergence.
The donor run now keeps assignments, objectives, and coordinates finite. Donor and 10x chemistry cases also match the multiplicative assignment formula.
At
sigma=0.1, the maximum assignment change is1.17e-6. The maximum corrected-coordinate change is1.15e-7.The existing test suite passes.
R CMD check --no-manual --timingscompletes withStatus: OK.