Skip to content

fix(cytovi): stabilize log-median probability aggregation - #4040

Closed
sofoklis326 wants to merge 1 commit into
scverse:mainfrom
sofoklis326:fix/cytovi-log-median-stability
Closed

sofoklis326 wants to merge 1 commit into
scverse:mainfrom
sofoklis326:fix/cytovi-log-median-stability

Conversation

@sofoklis326

Copy link
Copy Markdown

CytoVI's log_median() currently computes:

np.log(np.median(np.exp(x), axis=axis))

The input already contains log probabilities. Exponentiating very
negative values underflows to zero, while very positive values overflow
to infinity. This can produce non-finite differential-abundance scores.

For example, groups with constant log scores of -900 and -880 should
produce a DA difference of -20. The current calculation instead produces
-inf - (-inf), yielding NaN.

This PR selects the central values directly in log space and combines
them using np.logaddexp. It preserves the probability-space median
definition for both odd and even sample counts.

Compatibility checks cover aggregation axes, missing values, infinities,
empty reductions, and preservation of the input array.

Validation:

  • Added 37 focused test cases.
  • On unmodified upstream, 16 cases fail and 21 pass.
  • With the fix, all 37 cases pass.
  • The existing CytoVI training smoke test also passes locally on CPU:
    38 passing tests in total.
  • Focused Ruff and formatting checks pass.
  • Added a changelog entry.

This fix is independent of the other CytoVI fixes proposed separately.

The full upstream CI matrix has not been run locally.

@ori-kron-wis

Copy link
Copy Markdown
Collaborator

was solved here: #4048

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants