Opened 8 years ago
Closed 7 years ago
#16888 closed defect (fixed)
weak popov form does not compute weak popov form
| Reported by: | ketzu | Owned by: | |
|---|---|---|---|
| Priority: | major | Milestone: | sage-6.6 |
| Component: | linear algebra | Keywords: | weak-popov-form matrix |
| Cc: | jsrn | Merged in: | |
| Authors: | David Mödinger | Reviewers: | David Lucas |
| Report Upstream: | N/A | Work issues: | |
| Branch: | c19cc11 (Commits, GitHub, GitLab) | Commit: | c19cc1182669d5534f1371dd42c5ddbcdf7875d7 |
| Dependencies: | Stopgaps: |
Status badges
Description (last modified by )
While working on the sage.matrix.matrix2.weak_popov_form (for sage.matrix.matrix_misc.weak_popov_form applies the same) method for performance issues I noticed something.
The weak Popov form as defined in [MS] is not computed by this method. The other references do not call this form weak Popov form, it is a les restrictive definition for a certain row reduced form of matrix.
Followup ticket for reimplementation of wpf: #16742 and #16896.
[MS] T. Mulders, A. Storjohann, "On lattice reduction for polynomial
matrices," J. Symbolic Comput. 35 (2003), no. 4, 377--401
Comment of weak_popov_form:
OUTPUT:
A 3-tuple `(W,N,d)` consisting of:
1. `W` - a matrix over `k(x)` giving a weak the Popov form of self
2. `N` - a matrix over `k[x]` representing row operations used to
transform `self` to `W`
3. `d` - degree of respective columns of W; the degree of a column is
the maximum of the degree of its elements
Change History (16)
comment:1 Changed 8 years ago by
- Cc jsrn added
- Component changed from PLEASE CHANGE to linear algebra
- Description modified (diff)
- Keywords weak-popov-form matrix added
- Type changed from PLEASE CHANGE to defect
comment:2 Changed 8 years ago by
- Description modified (diff)
comment:3 Changed 8 years ago by
- Branch set to u/ketzu/weak_popov_form_does_not_compute_weak_popov_form
comment:4 Changed 8 years ago by
- Commit set to 970fde5ed4df264be62bc54c7b09e75e56643a08
comment:5 Changed 8 years ago by
- Commit changed from 970fde5ed4df264be62bc54c7b09e75e56643a08 to 12755f174d1775e53ebf719612f5e984d7120677
Branch pushed to git repo; I updated commit sha1. New commits:
| 12755f1 | Reduced to be a renaming only.
|
comment:6 Changed 8 years ago by
- Description modified (diff)
comment:7 Changed 8 years ago by
See also #16896
comment:8 Changed 8 years ago by
- Status changed from new to needs_review
comment:9 Changed 8 years ago by
- Status changed from needs_review to needs_work
one doctest is failing
comment:10 Changed 8 years ago by
- Commit changed from 12755f174d1775e53ebf719612f5e984d7120677 to 78209179e32bead65ec5f3de09c343dc1d7ed297
Branch pushed to git repo; I updated commit sha1. New commits:
| 7820917 | Fixed the doctest of matrix/matrix_misc.py.
|
comment:11 Changed 8 years ago by
- Status changed from needs_work to needs_review
Unfortunately I was sick for a long time, so i fixed the commit.
comment:12 Changed 7 years ago by
- Branch changed from u/ketzu/weak_popov_form_does_not_compute_weak_popov_form to u/dlucas/weak_popov_form_does_not_compute_weak_popov_form
comment:13 Changed 7 years ago by
- Commit changed from 78209179e32bead65ec5f3de09c343dc1d7ed297 to c19cc1182669d5534f1371dd42c5ddbcdf7875d7
I just changed some minor things into documentation : I removed any reference related to weak Popov form, because it does not compute the weak Popov form and I added a new reference related to the row reduced form. I also removed the name "weak Popov form" from the output field.
comment:14 Changed 7 years ago by
- Reviewers set to David Lucas
- Status changed from needs_review to positive_review
comment:15 Changed 7 years ago by
- Milestone changed from sage-6.4 to sage-6.6
comment:16 Changed 7 years ago by
- Branch changed from u/dlucas/weak_popov_form_does_not_compute_weak_popov_form to c19cc1182669d5534f1371dd42c5ddbcdf7875d7
- Resolution set to fixed
- Status changed from positive_review to closed
I think there should be a keyword ("transposition=False" oder something) to indicate the matrix N is wanted. Also I am not completely sure I did the deprecations right in any way.
New commits:
Rename of weak_popov_form() to row_reduced_form() added deprecation warning, rework of the use of the ascend parameter, removed sorting and returning d if ascend is not set.