Skip to content

Fix KL divergence clipping for integer regression inputs - #1348

Merged
solarsys merged 1 commit into
sunlabuiuc:masterfrom
pemalama12:fix/integer-kl-divergence
Oct 30, 2026
Merged

solarsys merged 1 commit into
sunlabuiuc:masterfrom
pemalama12:fix/integer-kl-divergence

Conversation

@pemalama12

Copy link
Copy Markdown
Contributor

Contributor: Pema Sangpo Lama (@pemalama12)

Contribution Type: Bug fix with regression tests

Description: Integer arrays containing zeros produce NaN for KL divergence: assigning the 1e-6 floor into an integer copy truncates it to zero. Use np.clip to create copies that can represent the floor. This also fixes the time-series wrapper while preserving the inputs and MSE/MAE values.

Files to Review:

  • pyhealth/metrics/regression.py: clipping fix.
  • tests/core/test_regression_integer_kl.py: signed/unsigned integers, mixed dtypes, all-zero arrays, float compatibility, metric order, unchanged inputs, and the time-series wrapper.

Validation: 15 integer cases fail on master; all 5 new tests and 10 existing time-series tests pass with the fix. Ruff passes for the new tests, and tools/check_pr_rules.py passes.

Manual reproduction:

import numpy as np
from pyhealth.metrics import regression_metrics_fn

scores = regression_metrics_fn(np.array([0, 1]), np.array([1, 0]))
print(scores)

Expected: finite KL divergence ≈ 13.81548, MSE 1.0, MAE 1.0. Master returns NaN for KL.

Separate from #1324's docstring and shape-check changes.

@github-actions

Copy link
Copy Markdown

Test Coverage

Total: 73.30% (19606 / 26747 lines)

Modified file Coverage Lines
pyhealth/metrics/regression.py 85.29% 29 / 34

Workflow run · 10696dc

@solarsys solarsys left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the diff and ran the full core suite locally with this PR applied; no new failures. Thanks!

@solarsys
solarsys merged commit 2c8fc06 into sunlabuiuc:master Oct 30, 2026
2 checks passed
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