Opened 6 years ago

Closed 6 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) Commit: c19cc1182669d5534f1371dd42c5ddbcdf7875d7
Dependencies: Stopgaps:

Description (last modified by ketzu)

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 6 years ago by ketzu

  • 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 6 years ago by ketzu

  • Description modified (diff)

comment:3 Changed 6 years ago by ketzu

  • Branch set to u/ketzu/weak_popov_form_does_not_compute_weak_popov_form

comment:4 Changed 6 years ago by ketzu

  • Commit set to 970fde5ed4df264be62bc54c7b09e75e56643a08

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:

970fde5Rename 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.

comment:5 Changed 6 years ago by git

  • Commit changed from 970fde5ed4df264be62bc54c7b09e75e56643a08 to 12755f174d1775e53ebf719612f5e984d7120677

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

12755f1Reduced to be a renaming only.

comment:6 Changed 6 years ago by ketzu

  • Authors set to David Mödinger
  • Description modified (diff)

New commits:

12755f1Reduced to be a renaming only.

New commits:

12755f1Reduced to be a renaming only.

comment:7 Changed 6 years ago by ketzu

See also #16896

comment:8 Changed 6 years ago by ketzu

  • Status changed from new to needs_review

comment:9 Changed 6 years ago by chapoton

  • Status changed from needs_review to needs_work

one doctest is failing

comment:10 Changed 6 years ago by git

  • Commit changed from 12755f174d1775e53ebf719612f5e984d7120677 to 78209179e32bead65ec5f3de09c343dc1d7ed297

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

7820917Fixed the doctest of matrix/matrix_misc.py.

comment:11 Changed 6 years ago by ketzu

  • Status changed from needs_work to needs_review

Unfortunately I was sick for a long time, so i fixed the commit.

comment:12 Changed 6 years ago by dlucas

  • 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 6 years ago by dlucas

  • 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 6 years ago by dlucas

  • Reviewers set to David Lucas
  • Status changed from needs_review to positive_review

comment:15 Changed 6 years ago by chapoton

  • Milestone changed from sage-6.4 to sage-6.6

comment:16 Changed 6 years ago by vbraun

  • 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
Note: See TracTickets for help on using tickets.