Opened 3 years ago

Closed 3 years ago

#26356 closed enhancement (fixed)

Minimal approximant bases

Reported by: vneiger Owned by: vneiger
Priority: major Milestone: sage-8.7
Component: algebra Keywords: polynomial matrix; approximant basis; Hermite-Padé approximation
Cc: gh-romainlebreton Merged in:
Authors: Seung Gyu Hyun, Vincent Neiger Reviewers: Romain Lebreton, Pascal Giorgi
Report Upstream: N/A Work issues:
Branch: fe6472e (Commits, GitHub, GitLab) Commit: fe6472ef2b732ad09d82971dfb538d92feaf53b7
Dependencies: Stopgaps:

Status badges

Description

New functionalities:

  • computation of shifted minimal approximant bases (iterative algorithm),
  • verification that a matrix is a shifted minimal approximant basis.

This should be done in a general context:

  • accepting non-uniform orders and non-uniform shifts,
  • allowing to work row-wise or column-wise,
  • offering the possibility of obtaining the canonical basis (that is, the one in shifted Popov form).

Change History (19)

comment:1 Changed 3 years ago by vneiger

  • Branch set to u/vneiger/minimal_approximant_bases

comment:2 Changed 3 years ago by git

  • Commit set to c25863aa7eab0f003fa19f5095b37eeb79107d28

Branch pushed to git repo; I updated commit sha1. New commits:

4099a15first version of approximant_basis including normal_form management
610a29afirst version of is_approximant_basis, just missing generation verification
17deb1csome cleaning and more examples in is_minimal_approx_basis
9c3d757cleaning the doc
0b0b0ffadd some examples for approximant_basis
c25863afinished a first working clean version of minimal approximant basis (+ verification)

comment:3 Changed 3 years ago by vneiger

The features mentioned in the ticket have been implemented, with complete documentation and tests. After a few more tests, for safety, this will be ready for review.

comment:4 Changed 3 years ago by git

  • Commit changed from c25863aa7eab0f003fa19f5095b37eeb79107d28 to 00238d638be9f449262e7445e3ab7760ff2d148e

Branch pushed to git repo; I updated commit sha1. New commits:

00238d6add info for changes on top of file

comment:5 Changed 3 years ago by git

  • Commit changed from 00238d638be9f449262e7445e3ab7760ff2d148e to b300f4fb2c5eb2cf5f11cf0b82be9fd4af599fe0

Branch pushed to git repo; I updated commit sha1. New commits:

b300f4fsimplification of tests for actual implementation of approx_basis

comment:6 Changed 3 years ago by vneiger

  • Owner changed from (none) to vneiger

comment:7 Changed 3 years ago by vneiger

  • Status changed from new to needs_review

Code is ready for review. Thank you for suggestions on improvements.

comment:8 Changed 3 years ago by git

  • Commit changed from b300f4fb2c5eb2cf5f11cf0b82be9fd4af599fe0 to 667404643b586ab976799a4bb0cadc00df430078

Branch pushed to git repo; I updated commit sha1. New commits:

6674046minor correction in link to bibliography

comment:9 Changed 3 years ago by gh-romainlebreton

  • Branch changed from u/vneiger/minimal_approximant_bases to u/gh-romainlebreton/minimal_approximant_bases

comment:10 Changed 3 years ago by git

  • Commit changed from 667404643b586ab976799a4bb0cadc00df430078 to a6d0b4b4567dc8eda6f3011a3866939544afb71e

Branch pushed to git repo; I updated commit sha1. New commits:

a6d0b4bMinor mdofications:

comment:11 Changed 3 years ago by git

  • Commit changed from a6d0b4b4567dc8eda6f3011a3866939544afb71e to 86e5225b64a33e7ce58763ec4071e6d10fe1baba

Branch pushed to git repo; I updated commit sha1. New commits:

088a0a5Fix rest_order positiveness in _approximant_basis_iterative
2431f90Merge tag '8.7.beta2' into u/vneiger/minimal_approximant_bases
86e5225Fix multiline ValueError

comment:12 Changed 3 years ago by gh-romainlebreton

Hello vneiger,

your code was great. I have only noticed a few problems that I solved. Could you please double check my changes ? If everything is fine by you and if the continuous integration passes, then you can mark it as a positive review. Best, Romain

comment:13 Changed 3 years ago by gh-romainlebreton

Quick question : why the function names is_minimal_approximant_basis and approximant_basis differ ? Should we rename the 2nd function minimal_approximant_basis ?

comment:14 Changed 3 years ago by vneiger

Hi,

Thank you very much for your reviewing and improvements of the code. All this looks good to me now.

I would only suggest to indeed change the name to "minimal_approximant_basis", and to replace "order should be positive integers" with "order should consist of positive integers". May I let you do this? For some reason, with the branch change you made, I have difficulties pushing my changes (I get in detached HEAD for a reason I'm not sure to understand...).

Thank you.

comment:15 Changed 3 years ago by git

  • Commit changed from 86e5225b64a33e7ce58763ec4071e6d10fe1baba to fe6472ef2b732ad09d82971dfb538d92feaf53b7

Branch pushed to git repo; I updated commit sha1. New commits:

fe6472eRename approximant_basis into minimal_approximant_basis

comment:16 Changed 3 years ago by gh-romainlebreton

Done ! LGTM

comment:17 Changed 3 years ago by vneiger

  • Milestone changed from sage-8.4 to sage-8.7
  • Status changed from needs_review to positive_review

comment:18 Changed 3 years ago by vneiger

Thank you for your work!

comment:19 Changed 3 years ago by vbraun

  • Branch changed from u/gh-romainlebreton/minimal_approximant_bases to fe6472ef2b732ad09d82971dfb538d92feaf53b7
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.