Opened 5 years ago

Last modified 2 months ago

#23486 needs_work enhancement

Echelon form for p-adic matrices

Reported by: Xavier Caruso Owned by:
Priority: major Milestone: sage-9.8
Component: padics Keywords: sd87, padicIMA, padicBordeaux
Cc: David Roe, Julian Rüth, Tristan Vaccon Merged in:
Authors: Xavier Caruso Reviewers: Ben Hutz, David Roe
Report Upstream: N/A Work issues:
Branch: u/caruso/padic_hermite (Commits, GitHub, GitLab) Commit: 26f0fd223edb143de3465f454e2dde5be1c06f9c
Dependencies: #23450 Stopgaps:

Status badges

Description

This ticket implements echelon form for matrices over complete discrete valuation rings/fields.

Change History (69)

comment:1 Changed 5 years ago by Xavier Caruso

Branch: u/caruso/padic_hermite

comment:2 Changed 5 years ago by Xavier Caruso

Commit: 013880536ee5ec0499dcd749d58f3b36ec6b71cd
Dependencies: #23450

New commits:

879f4ebSmith form of a p-adic matrix
595bc11Code moved into the category
1832757Factorisation of code
e2cdd99Small fix
7ae1018Move relevant code to sage.matrix.matrix_cdv_dense
7e73fd2Fix import
1fd67feAdd method tracks_precision()
0138805Echelon form for matrices over CDVR/CDVF

comment:3 Changed 5 years ago by git

Commit: 013880536ee5ec0499dcd749d58f3b36ec6b71cd02aa72a42558cad734be80d626af5552bd44751a

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

1f731eeCheck correctly (hopefully) that precision on input is enough
1f07da5Small fixes
e265c77Merge branch 'padic_smith' into padic_hermite
0c3e7ccFix precision issues
55b99dfSmall fix in lift_to_maximal_precision
1c396e8Merge branch 'padic_smith' into padic_hermite
71c0c74Add doctest
731fb19Fix doctest
1ea48acTwo small bugs fixed
02aa72aMerge branch 'padic_smith' into padic_hermite

comment:4 Changed 5 years ago by Xavier Caruso

Ticket ready for review!

comment:5 Changed 5 years ago by Xavier Caruso

Status: newneeds_review

comment:6 Changed 5 years ago by Ben Hutz

Reviewers: Ben Hutz
Status: needs_reviewneeds_work

I looked at this from a functionality perspective as opposed to looking through the algorithm line by line.

All of the examples I was able to come up with behaved as expected, so I just have a few minor issues

  • 2 doc test failures
    sage -t --warn-long 72.0 rings/laurent_series_ring.py  # 1 doctest failed
    sage -t --warn-long 72.0 rings/padics/tests.py  # 1 doctest failed
    
  • in a couple places you say
    if `d_i` denotes the `(i,i)` entry of `D`
    

I think you means D to be S

  • extra word
    'it is possible to do not'
    
  • in several places you say 'also possible for rectangular matrices' where you mean 'nonrectangular'
  • in two different places for echelon form you state it as find the Smith normal form instead of the echelon form
    to determine the Smith normal form
    
  • is there a reason to assign
        else:
            left = right = None
    

they don't seem to be used later.

  • incorrect word
integer this of this

should be 'integer ring of this'

comment:7 Changed 5 years ago by git

Commit: 02aa72a42558cad734be80d626af5552bd44751aba4a937c4a44536da0273d736000619476707206

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

52a4344Added keyword secure
65ffbc6Use the name "integral_smith_form" for matrices over CDVF
f27820aSmall fixes
ba4a937Merge branch 'padic_smith' into padic_hermite

comment:8 Changed 5 years ago by Xavier Caruso

Thanks for your comments.

I fixed the wording. The failing doctests were due to the fact that, in the case of a matrix M defined over a complete discrete valuation *field* K, my function echelon_form() did not compute the row-echelon from of M over K but over its ring of integers (i.e. the transformation matrix has coefficients in the ring of integers and is invertible over it). I then changed the name for integral_echelon_from().

NB: Some of your comments are related to ticket #23450. Maybe you should look at this one as well.

comment:9 Changed 5 years ago by Xavier Caruso

Status: needs_workneeds_review

comment:10 Changed 5 years ago by David Roe

Branch: u/caruso/padic_hermiteu/roed/padic_hermite

comment:11 Changed 5 years ago by David Roe

Commit: ba4a937c4a44536da0273d736000619476707206190ba1bf1b68eb012e7b42900a7fd50096860b07

See also #17272, which just got a positive review.


New commits:

190ba1bMerge branch 'u/caruso/padic_hermite' of git://trac.sagemath.org/sage into t/23486/padic_echelon

comment:12 Changed 5 years ago by Xavier Caruso

Great! So, what do we do with this ticket (which is now a kind of redundant)?

comment:13 Changed 5 years ago by David Roe

Are there any features here that aren't included in #17272?

comment:14 Changed 5 years ago by Xavier Caruso

The computation of transformation matrices, I think.

Moreover, for matrices over Qp, it makes sense to have an option for computing the echelon form over Zp (that is, without renormalizing the pivots) in order to save precision. But I don't think that it's implemented in this ticket.

comment:15 Changed 4 years ago by David Roe

Keywords: padicIMA added

comment:16 Changed 4 years ago by Xavier Caruso

Branch: u/roed/padic_hermiteu/caruso/padic_hermite

comment:17 Changed 4 years ago by Xavier Caruso

Commit: 190ba1bf1b68eb012e7b42900a7fd50096860b078bf47ee19f44ee40782082fcbff5c09b76e66d17
Status: needs_reviewneeds_work

New commits:

8bf47eeMerge branch '8.3.rc1' into t/23486/padic_hermite

comment:18 Changed 3 years ago by David Roe

Keywords: padicBordeaux added

comment:19 Changed 3 years ago by David Roe

Branch: u/caruso/padic_hermiteu/roed/padic_hermite

comment:20 Changed 3 years ago by David Roe

Commit: 8bf47ee19f44ee40782082fcbff5c09b76e66d17e522c94b89719bc51d7069cf0b682968814ff449

I merged in the latest beta, but this still needs to be reconciled with #17272. I think there are features here that we want that weren't done by #17272 (for example, when the base ring is Zp), so this ticket should remain open.


New commits:

e522c94Merge branch 'u/caruso/padic_hermite' of git://trac.sagemath.org/sage into 23486_padic_hermite

comment:21 Changed 10 months ago by Xavier Caruso

Branch: u/roed/padic_hermiteu/caruso/padic_hermite

comment:22 Changed 10 months ago by Xavier Caruso

Commit: e522c94b89719bc51d7069cf0b682968814ff449d4e1d54fe4d4de8da62fe5a478a98091bb84cbfa

I noticed that there is a lot of variations between the way operations over matrices are designed:

  • echelon_form is directly implemented in the class sage.matrix.matrix2.Matrix
  • smith_form calls the method _matrix_smith_form of the base ring; concretely, this method is implemented in the class sage.rings.padics.local_generic.LocalGeneric
  • hessenberg_form calls the method _matrix_hessenbergize of the base ring; this method is defined in the category sage.categories.discrete_valuation.DiscreteValuationFields and concretely implemented in the separate file sage.matrix.matrix_cdv.

I think it would be a good thing to uniformize all of this and, personally, I have a small preference for the third approach. What's your opinion?


New commits:

d4e1d54Merge branch 'u/roed/padic_hermite' of git://trac.sagemath.org/sage into echelon_form

comment:23 Changed 10 months ago by git

Commit: d4e1d54fe4d4de8da62fe5a478a98091bb84cbfa20db181c70ca931f91b82fdf6d22a867a1cc718a

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

20db181Merge branch 'develop' into echelon_form

comment:24 Changed 10 months ago by git

Commit: 20db181c70ca931f91b82fdf6d22a867a1cc718a0b3920e84f24497bd6fc2149c9f3a3271121f186

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

6935c3crefactor smith form and Hermite form
5087debechelon form
0c14b2aadd documentation
0b3920eadd function _matrix_determinant

comment:25 Changed 10 months ago by Xavier Caruso

Status: needs_workneeds_review

I followed the third approach: I put everything in the category and put the code in a separate pyx file. So far, most of the code is written in pure python; maybe we should move to cython at some point.

There is some code duplication due to the fact that we have separate categories for discrete valuation rings and fields but not a common category for both.

Anyway, everything is supposed over all types of p-adics as well as over power series and Laurent series rings. Moreover, with the general machinery already provided by sage, this is enough to manipulate free modules over those rings, including then lattices in finite dimensional p-adic vector spaces.

Last edited 10 months ago by Xavier Caruso (previous) (diff)

comment:26 Changed 10 months ago by git

Commit: 0b3920e84f24497bd6fc2149c9f3a3271121f18659a46323ac01513a7283237b85a73d99debdfbc5

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

59a4632remove obsolete file matrix_cdv_dense.py

comment:27 Changed 10 months ago by Julian Rüth

Have you considered putting things into a .py file instead? I don't think that it makes a real difference in terms of performance (and I am struggling right now to debug an (unrelated) call that uses too much Cython ;) )

comment:28 Changed 10 months ago by Xavier Caruso

The point is that matrix_cdv.pyx already exists with some cython code inside. But I can probably split the file in two parts and call them matrix_cdv_python and matrix_cdv_cython.

comment:29 Changed 9 months ago by Xavier Caruso

Status: needs_reviewneeds_work

The patchbot reported a lot of doctest failures... :-(

comment:30 Changed 9 months ago by git

Commit: 59a46323ac01513a7283237b85a73d99debdfbc5832813b29edc4bb279853c6f7c56fff82c0ee155

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

832813braise a TypeError instead of an ArithmeticError

comment:31 Changed 9 months ago by Xavier Caruso

Status: needs_workneeds_review

comment:32 Changed 9 months ago by git

Commit: 832813b29edc4bb279853c6f7c56fff82c0ee1552cffd3f000e6a17797fe80e235d6b6f73cd5f632

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

2cffd3ffix pyflakes

comment:33 Changed 9 months ago by git

Commit: 2cffd3f000e6a17797fe80e235d6b6f73cd5f632ac8abe96cf5eb30f9c7138097d615effc33a4efb

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

ac8abe9more doctests

comment:34 Changed 9 months ago by git

Commit: ac8abe96cf5eb30f9c7138097d615effc33a4efbdc9f77f45b77ba70e2ba8e86779389c8d30ef1fc

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

dc9f77fminor fixes

comment:35 Changed 9 months ago by git

Commit: dc9f77f45b77ba70e2ba8e86779389c8d30ef1fc6d7e43e8134c061b464c86cd7e085637456b61ce

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

6d7e43eMerge branch 'develop' into echelon_form

comment:36 Changed 9 months ago by David Roe

Here are the test failures when I ran tests locally:

Doctesting 7 files using 64 threads.
sage -t --long --warn-long 40.9 --random-seed=317472647872885548861559054668703098865 src/sage/matrix/matrix_cdv.pxd
    [0 tests, 0.00 s]
sage -t --long --warn-long 40.9 --random-seed=317472647872885548861559054668703098865 src/sage/matrix/matrix2.pxd
    [0 tests, 0.00 s]
sage -t --long --warn-long 40.9 --random-seed=317472647872885548861559054668703098865 src/sage/matrix/matrix_cdv.pyx
**********************************************************************
File "src/sage/matrix/matrix_cdv.pyx", line 710, in sage.matrix.matrix_cdv.hessenbergize_cdvf
Failed example:
    M.charpoly()
Expected:
    x^3 + (1 + 4*t + 4*t^2 + O(t^3))*x^2 + (t + 2*t^2 + O(t^3))*x + 3 + 2*t^2 + O(t^3)
Got:
    x^3 + (1 + 4*t + 4*t^2 + O(t^4))*x^2 + (t + 2*t^2 + O(t^3))*x + 3 + 2*t^2 + O(t^3)
**********************************************************************
1 item had failures:
   1 of  11 in sage.matrix.matrix_cdv.hessenbergize_cdvf
    [41 tests, 1 failure, 0.08 s]
sage -t --long --warn-long 40.9 --random-seed=317472647872885548861559054668703098865 src/sage/rings/laurent_series_ring.py
**********************************************************************
File "src/sage/rings/laurent_series_ring.py", line 119, in sage.rings.laurent_series_ring.LaurentSeriesRing
Failed example:
    TestSuite(F).run()
Expected nothing
Got:
    Failure in _test_euclidean_degree:
    Traceback (most recent call last):
      File "/home/roed/sage-9.6.beta2/local/var/lib/sage/venv-python3.8/lib/python3.8/site-packages/sage/misc/sage_unittest.py", line 297, in run
        test_method(tester=tester)
      File "/home/roed/sage-9.6.beta2/local/var/lib/sage/venv-python3.8/lib/python3.8/site-packages/sage/categories/euclidean_domains.py", line 155, in _test_euclidean_degree
        tester.assertEqual(a.euclidean_degree() == min_degree, a.is_unit())
      File "/usr/lib/python3.8/unittest/case.py", line 912, in assertEqual
        assertion_func(first, second, msg=msg)
      File "/usr/lib/python3.8/unittest/case.py", line 905, in _baseAssertEqual
        raise self.failureException(msg)
    AssertionError: False != True
    ------------------------------------------------------------
    Failure in _test_gcd_vs_xgcd:
    Traceback (most recent call last):
      File "/home/roed/sage-9.6.beta2/local/var/lib/sage/venv-python3.8/lib/python3.8/site-packages/sage/misc/sage_unittest.py", line 297, in run
        test_method(tester=tester)
      File "/home/roed/sage-9.6.beta2/local/var/lib/sage/venv-python3.8/lib/python3.8/site-packages/sage/categories/principal_ideal_domains.py", line 119, in _test_gcd_vs_xgcd
        tester.assertTrue(gcd==xgcd[0],
      File "/usr/lib/python3.8/unittest/case.py", line 765, in assertTrue
        raise self.failureException(msg)
    AssertionError: The methods gcd and xgcd disagree on Laurent Series Ring in x over Rational Field:
      gcd(x,x) = x
     xgcd(x,x) = (1, x^-1, 0)
    ------------------------------------------------------------
    Failure in _test_matrix_smith:
    Traceback (most recent call last):
      File "/home/roed/sage-9.6.beta2/local/var/lib/sage/venv-python3.8/lib/python3.8/site-packages/sage/misc/sage_unittest.py", line 297, in run
        test_method(tester=tester)
      File "/home/roed/sage-9.6.beta2/local/var/lib/sage/venv-python3.8/lib/python3.8/site-packages/sage/categories/complete_discrete_valuation.py", line 304, in _test_matrix_smith
        tester.assertEqual(U.base_ring(), base)
      File "/usr/lib/python3.8/unittest/case.py", line 912, in assertEqual
        assertion_func(first, second, msg=msg)
      File "/usr/lib/python3.8/unittest/case.py", line 905, in _baseAssertEqual
        raise self.failureException(msg)
    AssertionError: Laurent Series Ring in x over Rational Field != Power Series Ring in x over Rational Field
    ------------------------------------------------------------
    The following tests failed: _test_euclidean_degree, _test_gcd_vs_xgcd, _test_matrix_smith
**********************************************************************
File "src/sage/rings/laurent_series_ring.py", line 128, in sage.rings.laurent_series_ring.LaurentSeriesRing
Failed example:
    F.category()
Expected:
    Join of Category of complete discrete valuation fields and Category of commutative algebras over (finite enumerated fields and subquotients of monoids and quotients of semigroups) and Category of infinite sets
Got:
    Join of Category of complete discrete valuation rings and Category of commutative algebras over (finite enumerated fields and subquotients of monoids and quotients of semigroups) and Category of infinite sets
**********************************************************************
File "src/sage/rings/laurent_series_ring.py", line 130, in sage.rings.laurent_series_ring.LaurentSeriesRing
Failed example:
    TestSuite(F).run()
Expected nothing
Got:
    Failure in _test_euclidean_degree:
    Traceback (most recent call last):
      File "/home/roed/sage-9.6.beta2/local/var/lib/sage/venv-python3.8/lib/python3.8/site-packages/sage/misc/sage_unittest.py", line 297, in run
        test_method(tester=tester)
      File "/home/roed/sage-9.6.beta2/local/var/lib/sage/venv-python3.8/lib/python3.8/site-packages/sage/categories/euclidean_domains.py", line 155, in _test_euclidean_degree
        tester.assertEqual(a.euclidean_degree() == min_degree, a.is_unit())
      File "/usr/lib/python3.8/unittest/case.py", line 912, in assertEqual
        assertion_func(first, second, msg=msg)
      File "/usr/lib/python3.8/unittest/case.py", line 905, in _baseAssertEqual
        raise self.failureException(msg)
    AssertionError: False != True
    ------------------------------------------------------------
    Failure in _test_gcd_vs_xgcd:
    Traceback (most recent call last):
      File "/home/roed/sage-9.6.beta2/local/var/lib/sage/venv-python3.8/lib/python3.8/site-packages/sage/misc/sage_unittest.py", line 297, in run
        test_method(tester=tester)
      File "/home/roed/sage-9.6.beta2/local/var/lib/sage/venv-python3.8/lib/python3.8/site-packages/sage/categories/principal_ideal_domains.py", line 119, in _test_gcd_vs_xgcd
        tester.assertTrue(gcd==xgcd[0],
      File "/usr/lib/python3.8/unittest/case.py", line 765, in assertTrue
        raise self.failureException(msg)
    AssertionError: The methods gcd and xgcd disagree on Laurent Series Ring in x over Finite Field of size 11:
      gcd(x,x) = x
     xgcd(x,x) = (1, x^-1, 0)
    ------------------------------------------------------------
    Failure in _test_matrix_smith:
    Traceback (most recent call last):
      File "/home/roed/sage-9.6.beta2/local/var/lib/sage/venv-python3.8/lib/python3.8/site-packages/sage/misc/sage_unittest.py", line 297, in run
        test_method(tester=tester)
      File "/home/roed/sage-9.6.beta2/local/var/lib/sage/venv-python3.8/lib/python3.8/site-packages/sage/categories/complete_discrete_valuation.py", line 304, in _test_matrix_smith
        tester.assertEqual(U.base_ring(), base)
      File "/usr/lib/python3.8/unittest/case.py", line 912, in assertEqual
        assertion_func(first, second, msg=msg)
      File "/usr/lib/python3.8/unittest/case.py", line 905, in _baseAssertEqual
        raise self.failureException(msg)
    AssertionError: Laurent Series Ring in x over Finite Field of size 11 != Power Series Ring in x over Finite Field of size 11
    ------------------------------------------------------------
    The following tests failed: _test_euclidean_degree, _test_gcd_vs_xgcd, _test_matrix_smith
**********************************************************************
File "src/sage/rings/laurent_series_ring.py", line 151, in sage.rings.laurent_series_ring.LaurentSeriesRing
Failed example:
    LaurentSeriesRing(QQ, 'x').category()
Expected:
    Join of Category of complete discrete valuation fields and Category of commutative algebras over (number fields and quotient fields and metric spaces) and Category of infinite sets
Got:
    Join of Category of complete discrete valuation rings and Category of fields and Category of commutative algebras over (number fields and quotient fields and metric spaces) and Category of infinite sets
**********************************************************************
File "src/sage/rings/laurent_series_ring.py", line 227, in sage.rings.laurent_series_ring.LaurentSeriesRing.__init__
Failed example:
    R2.category()
Expected:
    Join of Category of complete discrete valuation fields and Category of commutative algebras over (finite enumerated fields and subquotients of monoids and quotients of semigroups) and Category of infinite sets
Got:
    Join of Category of complete discrete valuation rings and Category of commutative algebras over (finite enumerated fields and subquotients of monoids and quotients of semigroups) and Category of infinite sets
**********************************************************************
File "src/sage/rings/laurent_series_ring.py", line 229, in sage.rings.laurent_series_ring.LaurentSeriesRing.__init__
Failed example:
    TestSuite(R2).run()
Expected nothing
Got:
    Failure in _test_euclidean_degree:
    Traceback (most recent call last):
      File "/home/roed/sage-9.6.beta2/local/var/lib/sage/venv-python3.8/lib/python3.8/site-packages/sage/misc/sage_unittest.py", line 297, in run
        test_method(tester=tester)
      File "/home/roed/sage-9.6.beta2/local/var/lib/sage/venv-python3.8/lib/python3.8/site-packages/sage/categories/euclidean_domains.py", line 155, in _test_euclidean_degree
        tester.assertEqual(a.euclidean_degree() == min_degree, a.is_unit())
      File "/usr/lib/python3.8/unittest/case.py", line 912, in assertEqual
        assertion_func(first, second, msg=msg)
      File "/usr/lib/python3.8/unittest/case.py", line 905, in _baseAssertEqual
        raise self.failureException(msg)
    AssertionError: False != True
    ------------------------------------------------------------
    Failure in _test_gcd_vs_xgcd:
    Traceback (most recent call last):
      File "/home/roed/sage-9.6.beta2/local/var/lib/sage/venv-python3.8/lib/python3.8/site-packages/sage/misc/sage_unittest.py", line 297, in run
        test_method(tester=tester)
      File "/home/roed/sage-9.6.beta2/local/var/lib/sage/venv-python3.8/lib/python3.8/site-packages/sage/categories/principal_ideal_domains.py", line 119, in _test_gcd_vs_xgcd
        tester.assertTrue(gcd==xgcd[0],
      File "/usr/lib/python3.8/unittest/case.py", line 765, in assertTrue
        raise self.failureException(msg)
    AssertionError: The methods gcd and xgcd disagree on Laurent Series Ring in t over Ring of integers modulo 2:
      gcd(t,t) = t
     xgcd(t,t) = (1, t^-1, 0)
    ------------------------------------------------------------
    Failure in _test_matrix_smith:
    Traceback (most recent call last):
      File "/home/roed/sage-9.6.beta2/local/var/lib/sage/venv-python3.8/lib/python3.8/site-packages/sage/misc/sage_unittest.py", line 297, in run
        test_method(tester=tester)
      File "/home/roed/sage-9.6.beta2/local/var/lib/sage/venv-python3.8/lib/python3.8/site-packages/sage/categories/complete_discrete_valuation.py", line 304, in _test_matrix_smith
        tester.assertEqual(U.base_ring(), base)
      File "/usr/lib/python3.8/unittest/case.py", line 912, in assertEqual
        assertion_func(first, second, msg=msg)
      File "/usr/lib/python3.8/unittest/case.py", line 905, in _baseAssertEqual
        raise self.failureException(msg)
    AssertionError: Laurent Series Ring in t over Ring of integers modulo 2 != Power Series Ring in t over Ring of integers modulo 2
    ------------------------------------------------------------
    The following tests failed: _test_euclidean_degree, _test_gcd_vs_xgcd, _test_matrix_smith
**********************************************************************
File "src/sage/rings/laurent_series_ring.py", line 237, in sage.rings.laurent_series_ring.LaurentSeriesRing.__init__
Failed example:
    RQQ.category()
Expected:
    Join of Category of complete discrete valuation fields and Category of commutative algebras over (number fields and quotient fields and metric spaces) and Category of infinite sets
Got:
    Join of Category of complete discrete valuation rings and Category of commutative algebras over (number fields and quotient fields and metric spaces) and Category of infinite sets
**********************************************************************
File "src/sage/rings/laurent_series_ring.py", line 239, in sage.rings.laurent_series_ring.LaurentSeriesRing.__init__
Failed example:
    TestSuite(RQQ).run()
Expected nothing
Got:
    Failure in _test_euclidean_degree:
    Traceback (most recent call last):
      File "/home/roed/sage-9.6.beta2/local/var/lib/sage/venv-python3.8/lib/python3.8/site-packages/sage/misc/sage_unittest.py", line 297, in run
        test_method(tester=tester)
      File "/home/roed/sage-9.6.beta2/local/var/lib/sage/venv-python3.8/lib/python3.8/site-packages/sage/categories/euclidean_domains.py", line 155, in _test_euclidean_degree
        tester.assertEqual(a.euclidean_degree() == min_degree, a.is_unit())
      File "/usr/lib/python3.8/unittest/case.py", line 912, in assertEqual
        assertion_func(first, second, msg=msg)
      File "/usr/lib/python3.8/unittest/case.py", line 905, in _baseAssertEqual
        raise self.failureException(msg)
    AssertionError: False != True
    ------------------------------------------------------------
    Failure in _test_gcd_vs_xgcd:
    Traceback (most recent call last):
      File "/home/roed/sage-9.6.beta2/local/var/lib/sage/venv-python3.8/lib/python3.8/site-packages/sage/misc/sage_unittest.py", line 297, in run
        test_method(tester=tester)
      File "/home/roed/sage-9.6.beta2/local/var/lib/sage/venv-python3.8/lib/python3.8/site-packages/sage/categories/principal_ideal_domains.py", line 119, in _test_gcd_vs_xgcd
        tester.assertTrue(gcd==xgcd[0],
      File "/usr/lib/python3.8/unittest/case.py", line 765, in assertTrue
        raise self.failureException(msg)
    AssertionError: The methods gcd and xgcd disagree on Laurent Series Ring in t over Rational Field:
      gcd(t,t) = t
     xgcd(t,t) = (1, t^-1, 0)
    ------------------------------------------------------------
    Failure in _test_matrix_smith:
    Traceback (most recent call last):
      File "/home/roed/sage-9.6.beta2/local/var/lib/sage/venv-python3.8/lib/python3.8/site-packages/sage/misc/sage_unittest.py", line 297, in run
        test_method(tester=tester)
      File "/home/roed/sage-9.6.beta2/local/var/lib/sage/venv-python3.8/lib/python3.8/site-packages/sage/categories/complete_discrete_valuation.py", line 304, in _test_matrix_smith
        tester.assertEqual(U.base_ring(), base)
      File "/usr/lib/python3.8/unittest/case.py", line 912, in assertEqual
        assertion_func(first, second, msg=msg)
      File "/usr/lib/python3.8/unittest/case.py", line 905, in _baseAssertEqual
        raise self.failureException(msg)
    AssertionError: Laurent Series Ring in t over Rational Field != Power Series Ring in t over Rational Field
    ------------------------------------------------------------
    The following tests failed: _test_euclidean_degree, _test_gcd_vs_xgcd, _test_matrix_smith
**********************************************************************
2 items had failures:
   4 of  43 in sage.rings.laurent_series_ring.LaurentSeriesRing
   4 of  18 in sage.rings.laurent_series_ring.LaurentSeriesRing.__init__
    [182 tests, 8 failures, 0.21 s]
sage -t --long --warn-long 40.9 --random-seed=317472647872885548861559054668703098865 src/sage/rings/multi_power_series_ring_element.py
**********************************************************************
File "src/sage/rings/multi_power_series_ring_element.py", line 1081, in sage.rings.multi_power_series_ring_element.MPowerSeries.__mod__
Failed example:
    g in R
Expected:
    False
Got:
    True
**********************************************************************
1 item had failures:
   1 of   7 in sage.rings.multi_power_series_ring_element.MPowerSeries.__mod__
    [466 tests, 1 failure, 0.27 s]
sage -t --long --warn-long 40.9 --random-seed=317472647872885548861559054668703098865 src/sage/categories/complete_discrete_valuation.py
**********************************************************************
File "src/sage/categories/complete_discrete_valuation.py", line 621, in sage.categories.complete_discrete_valuation.CompleteDiscreteValuationFields
Failed example:
    LaurentSeriesRing(QQ,'u') in CompleteDiscreteValuationFields()
Expected:
    True
Got:
    False
**********************************************************************
1 item had failures:
   1 of   6 in sage.categories.complete_discrete_valuation.CompleteDiscreteValuationFields
    [200 tests, 1 failure, 0.24 s]
sage -t --long --warn-long 40.9 --random-seed=317472647872885548861559054668703098865 src/sage/matrix/matrix2.pyx
**********************************************************************
File "src/sage/matrix/matrix2.pyx", line 9855, in sage.matrix.matrix2.Matrix.inverse
Failed example:
    ~M
Expected:
    [t^-1 t^-2]
    [   0 t^-1]
Got:
    [1 0]
    [0 1]
**********************************************************************
1 item had failures:
   1 of  13 in sage.matrix.matrix2.Matrix.inverse
    [2807 tests, 1 failure, 8.67 s]
----------------------------------------------------------------------
sage -t --long --warn-long 40.9 --random-seed=317472647872885548861559054668703098865 src/sage/matrix/matrix_cdv.pyx  # 1 doctest failed
sage -t --long --warn-long 40.9 --random-seed=317472647872885548861559054668703098865 src/sage/rings/laurent_series_ring.py  # 8 doctests failed
sage -t --long --warn-long 40.9 --random-seed=317472647872885548861559054668703098865 src/sage/rings/multi_power_series_ring_element.py  # 1 doctest failed
sage -t --long --warn-long 40.9 --random-seed=317472647872885548861559054668703098865 src/sage/categories/complete_discrete_valuation.py  # 1 doctest failed
sage -t --long --warn-long 40.9 --random-seed=317472647872885548861559054668703098865 src/sage/matrix/matrix2.pyx  # 1 doctest failed

comment:37 Changed 9 months ago by David Roe

Reviewers: Ben HutzBen Hutz, David Roe
Status: needs_reviewneeds_work

comment:38 Changed 9 months ago by git

Commit: 6d7e43e8134c061b464c86cd7e085637456b61ce6a2d2c932cf557410b1632c99e34bc070d790080

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

6a2d2c9Laurent series ring is a field

comment:39 Changed 9 months ago by Xavier Caruso

I fixed the doctest failures, except the one in src/sage/rings/multi_power_series_ring_element.py because I can't understand the relation with this ticket. Any idea?

comment:40 Changed 9 months ago by Xavier Caruso

Oh, it's just because I removed the function __contains__ in sage.rings.power_series_ring.

However, this function didn't work as expected. Typically, we had:

sage: R.<t> = QQ[[]]
sage: K = R.fraction_field()
sage: K(t) in R
False

Falling back to the generic implementation of __contains__, this problem disappears but an element in Z/2Z[[u]] is considered as belonging to Z[[u]]. It's certainly not mathematically correct but the same holds for polynomials. So I'm not sure if it's better to fix the bug (I don't expect this to be easy) or the doctest.

Last edited 9 months ago by Xavier Caruso (previous) (diff)

comment:41 Changed 9 months ago by David Roe

I think this is just a consequence of Sage's longstanding definition of containment. It's analogous to

sage: mod(3, 7) in ZZ                                                                                                  
True

So I would say that restoring the generic behavior for containment is correct.

comment:42 Changed 9 months ago by Xavier Caruso

OK. Do you agree if I just remove the failing doctest?

comment:43 Changed 9 months ago by David Roe

I agree. In fact, the implementation of __mod__ here does not agree with the convention in the rest of Sage, where __mod__ returns something in the same parent rather than just changing the base ring. But I think that's an issue for another ticket.

comment:44 Changed 9 months ago by git

Commit: 6a2d2c932cf557410b1632c99e34bc070d790080dc51f10b50bf804dca35589ee9f86adfeebaa8f2

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

dc51f10remove failing doctest

comment:45 Changed 9 months ago by Xavier Caruso

Status: needs_workneeds_review

comment:46 Changed 9 months ago by David Roe

Another comment (since I see you're still awake)

  • Anywhere you use backslashes in docstrings you need to make sure that they're raw strings (have an "r" before the triple quote)

I'm reading through the changes now and may have more comments....

comment:47 Changed 9 months ago by git

Commit: dc51f10b50bf804dca35589ee9f86adfeebaa8f257a41b22f7375eec096c2af524ba5b7c871ed58c

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

57a41b2small fix

comment:48 Changed 7 months ago by Frédéric Chapoton

Milestone: sage-8.1sage-9.7
Status: needs_reviewneeds_work

red branch => needs work

comment:49 Changed 6 months ago by git

Commit: 57a41b22f7375eec096c2af524ba5b7c871ed58cbd75b0dcfc6b0f450463cfa0103184a73fac1ffa

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

bd75b0dMerge branch 'develop' into echelon_form

comment:50 Changed 6 months ago by Xavier Caruso

Status: needs_workneeds_review

I fixed merging issues.

I put needs review in order to get feedback from the patchbot.

comment:51 Changed 6 months ago by git

Commit: bd75b0dcfc6b0f450463cfa0103184a73fac1ffa27b57b89b3b2b2962ead3a8b96468d5c0d943e82

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

27b57b8fix pyflakes

comment:52 Changed 6 months ago by Julian Rüth

Branch: u/caruso/padic_hermiteu/saraedum/padic_hermite

comment:53 Changed 6 months ago by git

Commit: 27b57b89b3b2b2962ead3a8b96468d5c0d943e82ecb4f01b734724d7f31e578a23fa374a08115ceb

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

ecb4f01Deduplicate code for complete discrete valuation rings and fields

comment:54 Changed 6 months ago by git

Commit: ecb4f01b734724d7f31e578a23fa374a08115ceb3c4cfd951811903df59b93ce7359ae6dacf3f934

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

9ec4bcfFix handling of transformation=false in echelonize
3c4cfd9Drop unused code

comment:55 Changed 6 months ago by git

Commit: 3c4cfd951811903df59b93ce7359ae6dacf3f934cb2404be69851ea823d1572504fd4001beafec64

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

2638908Fix LaTeX escaping in docstrings
0d8c61fImprove docstrings
941f1c0Drop commented out definitions
cb2404bRemove typo

comment:56 Changed 6 months ago by Julian Rüth

Are you using any results from the articles you wrote on p-adic precision? If so, I think that it would be nice to reference the articles in the ALGORITHM sections.


New commits:

eb97013Improve documentation of CompleteDiscreteValuationRings.ParentMethods
ecb4f01Deduplicate code for complete discrete valuation rings and fields
9ec4bcfFix handling of transformation=false in echelonize
3c4cfd9Drop unused code
2638908Fix LaTeX escaping in docstrings
0d8c61fImprove docstrings
941f1c0Drop commented out definitions
cb2404bRemove typo
Last edited 6 months ago by Julian Rüth (previous) (diff)

comment:57 Changed 6 months ago by Julian Rüth

How are these change related to this issue? Can they be reverted?

diff --git a/src/sage/rings/padics/relaxed_template.pxi b/src/sage/rings/padics/relaxed_template.pxi
index 7269da0..42af3ea 100644
--- a/src/sage/rings/padics/relaxed_template.pxi
+++ b/src/sage/rings/padics/relaxed_template.pxi
@@ -1265,19 +1265,10 @@ cdef class RelaxedElement(pAdicGenericElement):
             sage: a.lift_to_precision(20)
             4*5 + 4*5^2 + 5^4 + O(5^20)
 
-        When the precision is omitted, the default precision of the parent
-        is used::
+        When the precision is omitted, the element is made unbounded::
 
             sage: a.lift_to_precision()
-            4*5 + 4*5^2 + 5^4 + O(5^10)
-
-        When the parent is a field, the behaviour is slightly different since
-        the default precision of the parent becomes the relative precision
-        of the lifted element::
-
-            sage: K = R.fraction_field()
-            sage: K(a).lift_to_precision()
-            4*5 + 4*5^2 + 5^4 + O(5^11)
+            4*5 + 4*5^2 + 5^4 + ...
 
         Note that the precision never decreases::
 
@@ -1298,13 +1289,14 @@ cdef class RelaxedElement(pAdicGenericElement):
         cdef long prec
         cdef long default_prec = self._parent.default_prec()
         if absprec is None:
-            if self.prime_pow.in_field:
-                self._jump_relative_c(1, self._precbound)
-                if self._precrel == 0:
-                    return self._parent.zero()
-                prec = self._valuation + default_prec
-            else:
-                prec = default_prec
+            prec = maxordp
+            #if self.prime_pow.in_field:
+            #    self._jump_relative_c(1, self._precbound)
+            #    if self._precrel == 0:
+            #        return self._parent.zero()
+            #    prec = self._valuation + default_prec
+            #else:
+            #    prec = default_prec
         else:
             if absprec > maxordp:
                 raise OverflowError("beyond the maximal precision (which is %s)" % maxordp)

comment:58 Changed 6 months ago by git

Commit: cb2404be69851ea823d1572504fd4001beafec64ab32baca7f7aeae441a139fa4e1f1ed666ba5409

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

f595901Cleanup implementation of matrix_cdv
ab32bacRestore removed doctests

comment:59 Changed 6 months ago by Julian Rüth

Xavier, I made some review changes, trying to simplify things a bit and improving some docstrings (hopefully.)

This looks great to me, so once doctests pass again an the above comments have been adressed, this is good to be merged.


New commits:

f595901Cleanup implementation of matrix_cdv
ab32bacRestore removed doctests

comment:60 Changed 6 months ago by Xavier Caruso

Branch: u/saraedum/padic_hermiteu/caruso/padic_hermite

comment:61 Changed 6 months ago by Xavier Caruso

Commit: ab32baca7f7aeae441a139fa4e1f1ed666ba54091068baa478b35940165b106a2c7d361256d88d85

It seems that your class SharedParentMethods does not work.

sage: R = Zp(5, prec=10)
sage: R._matrix_echelonize
Traceback (most recent call last):
...
AttributeError: 'pAdicRingCappedRelative_with_category' object has no attribute '_matrix_echelonize'

New commits:

1068baafix small bug

comment:62 Changed 6 months ago by Xavier Caruso

I'm getting the following warning when Sage starts:

/home/xcaruso/sage/local/var/lib/sage/venv-python3.8/lib/python3.8/site-packages/sage/categories/category.py:1592: UserWarning: CompleteDiscreteValuationRings.ParentMethods should not have a super class

So I'm coming back to the previous version.

comment:63 Changed 6 months ago by git

Commit: 1068baa478b35940165b106a2c7d361256d88d852094339c2fcd4f26de9056193f199431c09b40f9

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

2094339Revert "Deduplicate code for complete discrete valuation rings and fields"

comment:64 Changed 6 months ago by git

Commit: 2094339c2fcd4f26de9056193f199431c09b40f950520b68c430047dad8c3702c0232c4531658ad8

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

50520b6do not use get_unsafe in py files

comment:65 Changed 6 months ago by Julian Rüth

Branch: u/caruso/padic_hermiteu/saraedum/padic_hermite

comment:66 Changed 6 months ago by git

Commit: 50520b68c430047dad8c3702c0232c4531658ad8e6e1606e281dc6ec165c88c784a63e1aee20fafa

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

e6e1606Fix syntax error

comment:67 Changed 6 months ago by Xavier Caruso

Branch: u/saraedum/padic_hermiteu/caruso/padic_hermite

comment:68 Changed 4 months ago by Frédéric Chapoton

Commit: e6e1606e281dc6ec165c88c784a63e1aee20fafa26f0fd223edb143de3465f454e2dde5be1c06f9c
Status: needs_reviewneeds_work

red branch => needs work


New commits:

26f0fd2tests pass

comment:69 Changed 2 months ago by Matthias Köppe

Milestone: sage-9.7sage-9.8
Note: See TracTickets for help on using tickets.