Skip to content

feat: implement Toeplitz and Hankel Procrustes#226

Open
Ao-chuba wants to merge 2 commits intotheochem:mainfrom
Ao-chuba:feat/toeplitz-procrustes
Open

feat: implement Toeplitz and Hankel Procrustes#226
Ao-chuba wants to merge 2 commits intotheochem:mainfrom
Ao-chuba:feat/toeplitz-procrustes

Conversation

@Ao-chuba
Copy link
Copy Markdown
Contributor

@Ao-chuba Ao-chuba commented Feb 18, 2026

This PR implements Toeplitz and Hankel matrix Procrustes algorithms using a least squares approach over the structured parameter space.
The changes made are :

  • Added procrustes/toeplitz.py with toeplitz() and hankel() solvers.
  • Added procrustes/test/test_toeplitz.py with comprehensive tests for exact recovery and structure validation.
  • Updated procrustes/__init__.py to export the new functions.

Using the reference Yang, J., & Deng, Y. (2013). "Procrustes Problems for General, Triangular, and Symmetric Toeplitz Matrices."
Journal of Applied Mathematics, 2013.
closes #84

@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@a0c02ce). Learn more about missing BASE report.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #226   +/-   ##
=======================================
  Coverage        ?   94.59%           
=======================================
  Files           ?       12           
  Lines           ?      869           
  Branches        ?        0           
=======================================
  Hits            ?      822           
  Misses          ?       47           
  Partials        ?        0           
Files with missing lines Coverage Δ
procrustes/__init__.py 100.00% <100.00%> (ø)
procrustes/toeplitz.py 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Ao-chuba Ao-chuba force-pushed the feat/toeplitz-procrustes branch 3 times, most recently from bcf7d94 to 745d583 Compare February 18, 2026 15:20
@Ao-chuba
Copy link
Copy Markdown
Contributor Author

@FanwangM here's my PR please look into it, whenever you get time.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR implements one-sided Toeplitz and Hankel Procrustes transformations by reformulating the constrained optimization problem as a linear least-squares problem over the structured parameter space. The implementation follows the approach described in Yang & Deng (2013).

Changes:

  • Added procrustes/toeplitz.py with toeplitz() and hankel() solvers that find optimal Toeplitz/Hankel transformation matrices
  • Added comprehensive test suite in procrustes/test/test_toeplitz.py covering exact recovery, structure validation, and error bounds
  • Updated procrustes/__init__.py to export the new functions

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
procrustes/toeplitz.py Implements Toeplitz and Hankel Procrustes solvers using linear operator construction and least-squares optimization
procrustes/test/test_toeplitz.py Comprehensive test suite validating exact recovery, structural properties, and comparison with generic Procrustes
procrustes/init.py Exports new toeplitz module functions

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread procrustes/toeplitz.py
Comment thread procrustes/toeplitz.py
Comment thread procrustes/test/test_toeplitz.py
Comment thread procrustes/test/test_toeplitz.py
- Add procrustes/toeplitz.py with toeplitz() and hankel() solvers
- Both use a least-squares approach over the structured parameter space:
  vec(A @ T) = M @ t, solved via numpy.linalg.lstsq
- Helper functions _build_toeplitz_operator, _build_hankel_operator,
  _params_to_toeplitz, _params_to_hankel kept private
- Add procrustes/test/test_toeplitz.py with 24 tests covering:
  exact recovery, structure validation, error vs generic, shape errors,
  and ProcrustesResult field checks
- Export toeplitz and hankel from procrustes/__init__.py
@Ao-chuba Ao-chuba force-pushed the feat/toeplitz-procrustes branch from 745d583 to 535e4de Compare February 18, 2026 17:22
@Ao-chuba
Copy link
Copy Markdown
Contributor Author

@FanwangM ,
I've implemented rectangular matrix test as suggested by copilot and all test pass. Regarding the spelling suggestion Copilot flagged that this PR uses "initial" while other files use "intial". "intial" seems like a typo so I've decided not to go along with copilot's suggestion here.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 9 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread procrustes/toeplitz.py Outdated
Comment thread procrustes/toeplitz.py Outdated
Comment thread procrustes/test/test_toeplitz.py
Comment thread procrustes/toeplitz.py
Comment thread procrustes/toeplitz.py
Comment thread procrustes/toeplitz.py
Comment thread procrustes/test/test_toeplitz.py
Comment thread procrustes/toeplitz.py
Comment thread procrustes/toeplitz.py
@Ao-chuba
Copy link
Copy Markdown
Contributor Author

As per the suggestions given by co-pilot, All four np.zeros initializations in _build_toeplitz_operator, _build_hankel_operator, _params_to_toeplitz, and _params_to_hankel have been updated to carry dtype=a.dtype or dtype=params.dtype respectively, ensuring complex inputs are never silently truncated at any stage of the pipeline. Two test names were also corrected test_toeplitz_error_leq_generic and test_hankel_error_leq_generic were renamed to _geq_generic to match the >= assertion they actually make. Also the Notes sections in both toeplitz() and hankel() docstrings were updated to replace "solving the normal equations" with "found via least-squares (using an SVD-based solver)", since np.linalg.lstsq uses SVD internally, not the normal equations method.

Regarding the suggestion to replace the dense operator M with a scipy.sparse matrix and an iterative solver, this is a valid point, as the current dense formulation has O(n³) memory for square inputs. However, this represents a significant scope change, requires new dependencies, and is inconsistent with how the rest of the procrustes library handles solvers. It would be better if a separate issue will be opened for this issue rather than including it in this PR.

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.

One-Sided Procrustes for Toeplitz matrices

2 participants