Skip to content

Migrate np.polyfit to np.polynomial.polynomial.polyfit#2984

Open
danstam wants to merge 4 commits intounitaryfoundation:mainfrom
danstam:fix/numpy-polynomial-migration
Open

Migrate np.polyfit to np.polynomial.polynomial.polyfit#2984
danstam wants to merge 4 commits intounitaryfoundation:mainfrom
danstam:fix/numpy-polynomial-migration

Conversation

@danstam
Copy link
Copy Markdown

@danstam danstam commented Apr 1, 2026

Fixes #2322

Replaces the deprecated np.polyfit API with np.polynomial.polynomial.polyfit throughout mitiq/zne/inference.py.

What changed and why

The main challenge was that the new API removed the cov=True option. The covariance matrix is now reconstructed manually using the weighted Vandermonde approach:

cov = (residuals / (n - deg - 1)) * inv((W @ V).T @ (W @ V))

One non-obvious issue: full=True silently suppresses RankWarning — numpy only raises it when full=False. A manual rank-deficiency check (rank < deg + 1) was added to preserve the existing ExtrapolationWarning behavior.

Other changes:

  • Fixed coefficient ordering reversal across all affected call sites — the new API returns coefficients in ascending order [a_0, a_1, ..., a_n] vs the old descending order [a_n, ..., a_1, a_0]
  • Migrated np.polyvalnp.polynomial.polynomial.polyval (argument order is reversed in the new API)
  • Removed [::-1] reversals in _ansatz_unknown and _ansatz_known closures
  • Updated hardcoded test assertions to match ascending coefficient order
  • Added cast(npt.NDArray[np.float64], ...) to satisfy mypy since numpy's type stubs for full=True return an imprecise type

All 216 ZNE tests pass. Ruff and mypy clean.

AI use disclosure

I used Claude Code to help navigate the codebase and verify the Vandermonde covariance math. All implementation decisions and code reviews were done by me iteratively.

Copy link
Copy Markdown
Member

@natestemen natestemen left a comment

Choose a reason for hiding this comment

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

I don't want this to be a breaking change, so can you make sure the coefficients are returned in the same order as previously?

@danstam
Copy link
Copy Markdown
Author

danstam commented Apr 2, 2026

Added a reversal on the return value to keep the original coefficient ordering.

@natestemen natestemen self-requested a review April 4, 2026 19:46
Comment thread mitiq/zne/tests/test_inference.py
Comment thread mitiq/zne/inference.py Outdated
@danstam
Copy link
Copy Markdown
Author

danstam commented Apr 11, 2026

@natestemen Let me know if you need anything else.

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.

Remove usage of np.polyfit

2 participants