Skip to content

Correct Lp norms of signed persistence landscapes - #112

Merged
catanzaromj merged 2 commits into
scikit-tda:masterfrom
antonsoo:fix/signed-landscape-norm
Oct 18, 2026
Merged

catanzaromj merged 2 commits into
scikit-tda:masterfrom
antonsoo:fix/signed-landscape-norm

Conversation

@antonsoo

@antonsoo antonsoo commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Landscape subtraction produces signed functions, but _p_norm can cancel their zero-crossing integrals or raise negative values to fractional powers. For critical points (0,0), (1,1), (2,-1), (3,1), (4,0), the L2 norm currently returns sqrt(2/3) instead of sqrt(4/3).

Integrate each segment from its endpoint magnitudes. This also avoids reconstructing heights from a potentially large coordinate intercept. Both exact and approximate landscapes use the correction. Add signed/fractional, translated-coordinate and diagram-difference regressions, plus a 0.4.1 patch release note.

Validation: 8 of the 12 new cases fail before; all 12 pass after. The complete suite on refreshed master passes 124 tests, with 19 warnings. Ruff format/lint, wheel/sdist builds and metadata checks pass.

An additional audit of 660 signed norms from recorded handwritten-digit diagrams corrects 143 errors, including 113 nonfinite results, against exact Fraction L2 integrals and adaptive quadrature for the other exponents. This reference integrates the returned piecewise-linear function; a separately observed exact-landscape construction discrepancy remains outside this norm correction.

@codecov

codecov Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 79.28%. Comparing base (084c381) to head (d3b61b3).

Files with missing lines Patch % Lines
persim/_version.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #112      +/-   ##
==========================================
+ Coverage   78.87%   79.28%   +0.40%     
==========================================
  Files          20       20              
  Lines        1482     1477       -5     
  Branches      271      271              
==========================================
+ Hits         1169     1171       +2     
+ Misses        235      229       -6     
+ Partials       78       77       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@catanzaromj

Copy link
Copy Markdown
Contributor

Hi @antonsoo, thanks for opening the PR and raising this! This is definitely a bug, as your example shows. Its interesting that the bug only shows when $p$ is even (so that $p+1$ is odd), since otherwise the discrepancy is cancelled away. I also appreciate you simplifying the logic around these calculations.

Can you bump the patch version and update RELEASES.txt to reflect this as well? Thanks

@antonsoo
antonsoo force-pushed the fix/signed-landscape-norm branch from f101a62 to d3b61b3 Compare October 17, 2026 17:44
@antonsoo

Copy link
Copy Markdown
Contributor Author

Bumped the patch version to 0.4.1 and added the entry to RELEASE.txt in d3b61b3, after rebasing onto current master. The full local suite passes, and all 660 recorded-diagram norm comparisons still agree with the independent integrals.

@catanzaromj
catanzaromj merged commit 00144a8 into scikit-tda:master Oct 18, 2026
12 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