Opened 5 years ago
Last modified 2 months ago
#23486 needs_work enhancement
Echelon form for padic matrices
Reported by:  Xavier Caruso  Owned by:  

Priority:  major  Milestone:  sage9.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: 
Description
This ticket implements echelon form for matrices over complete discrete valuation rings/fields.
Change History (69)
comment:1 Changed 5 years ago by
Branch:  → u/caruso/padic_hermite 

comment:2 Changed 5 years ago by
Commit:  → 013880536ee5ec0499dcd749d58f3b36ec6b71cd 

Dependencies:  → #23450 
comment:3 Changed 5 years ago by
Commit:  013880536ee5ec0499dcd749d58f3b36ec6b71cd → 02aa72a42558cad734be80d626af5552bd44751a 

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
1f731ee  Check correctly (hopefully) that precision on input is enough

1f07da5  Small fixes

e265c77  Merge branch 'padic_smith' into padic_hermite

0c3e7cc  Fix precision issues

55b99df  Small fix in lift_to_maximal_precision

1c396e8  Merge branch 'padic_smith' into padic_hermite

71c0c74  Add doctest

731fb19  Fix doctest

1ea48ac  Two small bugs fixed

02aa72a  Merge branch 'padic_smith' into padic_hermite

comment:5 Changed 5 years ago by
Status:  new → needs_review 

comment:6 Changed 5 years ago by
Reviewers:  → Ben Hutz 

Status:  needs_review → needs_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 warnlong 72.0 rings/laurent_series_ring.py # 1 doctest failed sage t warnlong 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
Commit:  02aa72a42558cad734be80d626af5552bd44751a → ba4a937c4a44536da0273d736000619476707206 

comment:8 Changed 5 years ago by
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 rowechelon 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
Status:  needs_work → needs_review 

comment:10 Changed 5 years ago by
Branch:  u/caruso/padic_hermite → u/roed/padic_hermite 

comment:11 Changed 5 years ago by
Commit:  ba4a937c4a44536da0273d736000619476707206 → 190ba1bf1b68eb012e7b42900a7fd50096860b07 

comment:12 Changed 5 years ago by
Great! So, what do we do with this ticket (which is now a kind of redundant)?
comment:14 Changed 5 years ago by
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
Keywords:  padicIMA added 

comment:16 Changed 4 years ago by
Branch:  u/roed/padic_hermite → u/caruso/padic_hermite 

comment:17 Changed 4 years ago by
Commit:  190ba1bf1b68eb012e7b42900a7fd50096860b07 → 8bf47ee19f44ee40782082fcbff5c09b76e66d17 

Status:  needs_review → needs_work 
New commits:
8bf47ee  Merge branch '8.3.rc1' into t/23486/padic_hermite

comment:18 Changed 3 years ago by
Keywords:  padicBordeaux added 

comment:19 Changed 3 years ago by
Branch:  u/caruso/padic_hermite → u/roed/padic_hermite 

comment:20 Changed 3 years ago by
Commit:  8bf47ee19f44ee40782082fcbff5c09b76e66d17 → e522c94b89719bc51d7069cf0b682968814ff449 

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:
e522c94  Merge branch 'u/caruso/padic_hermite' of git://trac.sagemath.org/sage into 23486_padic_hermite

comment:21 Changed 10 months ago by
Branch:  u/roed/padic_hermite → u/caruso/padic_hermite 

comment:22 Changed 10 months ago by
Commit:  e522c94b89719bc51d7069cf0b682968814ff449 → d4e1d54fe4d4de8da62fe5a478a98091bb84cbfa 

I noticed that there is a lot of variations between the way operations over matrices are designed:
echelon_form
is directly implemented in the classsage.matrix.matrix2.Matrix
smith_form
calls the method_matrix_smith_form
of the base ring; concretely, this method is implemented in the classsage.rings.padics.local_generic.LocalGeneric
hessenberg_form
calls the method_matrix_hessenbergize
of the base ring; this method is defined in the categorysage.categories.discrete_valuation.DiscreteValuationFields
and concretely implemented in the separate filesage.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:
d4e1d54  Merge branch 'u/roed/padic_hermite' of git://trac.sagemath.org/sage into echelon_form

comment:23 Changed 10 months ago by
Commit:  d4e1d54fe4d4de8da62fe5a478a98091bb84cbfa → 20db181c70ca931f91b82fdf6d22a867a1cc718a 

Branch pushed to git repo; I updated commit sha1. New commits:
20db181  Merge branch 'develop' into echelon_form

comment:24 Changed 10 months ago by
Commit:  20db181c70ca931f91b82fdf6d22a867a1cc718a → 0b3920e84f24497bd6fc2149c9f3a3271121f186 

comment:25 Changed 10 months ago by
Status:  needs_work → needs_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 padics 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 padic vector spaces.
comment:26 Changed 10 months ago by
Commit:  0b3920e84f24497bd6fc2149c9f3a3271121f186 → 59a46323ac01513a7283237b85a73d99debdfbc5 

Branch pushed to git repo; I updated commit sha1. New commits:
59a4632  remove obsolete file matrix_cdv_dense.py

comment:27 Changed 10 months ago by
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
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
Status:  needs_review → needs_work 

The patchbot reported a lot of doctest failures... :(
comment:30 Changed 9 months ago by
Commit:  59a46323ac01513a7283237b85a73d99debdfbc5 → 832813b29edc4bb279853c6f7c56fff82c0ee155 

Branch pushed to git repo; I updated commit sha1. New commits:
832813b  raise a TypeError instead of an ArithmeticError

comment:31 Changed 9 months ago by
Status:  needs_work → needs_review 

comment:32 Changed 9 months ago by
Commit:  832813b29edc4bb279853c6f7c56fff82c0ee155 → 2cffd3f000e6a17797fe80e235d6b6f73cd5f632 

Branch pushed to git repo; I updated commit sha1. New commits:
2cffd3f  fix pyflakes

comment:33 Changed 9 months ago by
Commit:  2cffd3f000e6a17797fe80e235d6b6f73cd5f632 → ac8abe96cf5eb30f9c7138097d615effc33a4efb 

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

comment:34 Changed 9 months ago by
Commit:  ac8abe96cf5eb30f9c7138097d615effc33a4efb → dc9f77f45b77ba70e2ba8e86779389c8d30ef1fc 

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

comment:35 Changed 9 months ago by
Commit:  dc9f77f45b77ba70e2ba8e86779389c8d30ef1fc → 6d7e43e8134c061b464c86cd7e085637456b61ce 

Branch pushed to git repo; I updated commit sha1. New commits:
6d7e43e  Merge branch 'develop' into echelon_form

comment:36 Changed 9 months ago by
Here are the test failures when I ran tests locally:
Doctesting 7 files using 64 threads. sage t long warnlong 40.9 randomseed=317472647872885548861559054668703098865 src/sage/matrix/matrix_cdv.pxd [0 tests, 0.00 s] sage t long warnlong 40.9 randomseed=317472647872885548861559054668703098865 src/sage/matrix/matrix2.pxd [0 tests, 0.00 s] sage t long warnlong 40.9 randomseed=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 warnlong 40.9 randomseed=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/sage9.6.beta2/local/var/lib/sage/venvpython3.8/lib/python3.8/sitepackages/sage/misc/sage_unittest.py", line 297, in run test_method(tester=tester) File "/home/roed/sage9.6.beta2/local/var/lib/sage/venvpython3.8/lib/python3.8/sitepackages/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/sage9.6.beta2/local/var/lib/sage/venvpython3.8/lib/python3.8/sitepackages/sage/misc/sage_unittest.py", line 297, in run test_method(tester=tester) File "/home/roed/sage9.6.beta2/local/var/lib/sage/venvpython3.8/lib/python3.8/sitepackages/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/sage9.6.beta2/local/var/lib/sage/venvpython3.8/lib/python3.8/sitepackages/sage/misc/sage_unittest.py", line 297, in run test_method(tester=tester) File "/home/roed/sage9.6.beta2/local/var/lib/sage/venvpython3.8/lib/python3.8/sitepackages/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/sage9.6.beta2/local/var/lib/sage/venvpython3.8/lib/python3.8/sitepackages/sage/misc/sage_unittest.py", line 297, in run test_method(tester=tester) File "/home/roed/sage9.6.beta2/local/var/lib/sage/venvpython3.8/lib/python3.8/sitepackages/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/sage9.6.beta2/local/var/lib/sage/venvpython3.8/lib/python3.8/sitepackages/sage/misc/sage_unittest.py", line 297, in run test_method(tester=tester) File "/home/roed/sage9.6.beta2/local/var/lib/sage/venvpython3.8/lib/python3.8/sitepackages/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/sage9.6.beta2/local/var/lib/sage/venvpython3.8/lib/python3.8/sitepackages/sage/misc/sage_unittest.py", line 297, in run test_method(tester=tester) File "/home/roed/sage9.6.beta2/local/var/lib/sage/venvpython3.8/lib/python3.8/sitepackages/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/sage9.6.beta2/local/var/lib/sage/venvpython3.8/lib/python3.8/sitepackages/sage/misc/sage_unittest.py", line 297, in run test_method(tester=tester) File "/home/roed/sage9.6.beta2/local/var/lib/sage/venvpython3.8/lib/python3.8/sitepackages/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/sage9.6.beta2/local/var/lib/sage/venvpython3.8/lib/python3.8/sitepackages/sage/misc/sage_unittest.py", line 297, in run test_method(tester=tester) File "/home/roed/sage9.6.beta2/local/var/lib/sage/venvpython3.8/lib/python3.8/sitepackages/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/sage9.6.beta2/local/var/lib/sage/venvpython3.8/lib/python3.8/sitepackages/sage/misc/sage_unittest.py", line 297, in run test_method(tester=tester) File "/home/roed/sage9.6.beta2/local/var/lib/sage/venvpython3.8/lib/python3.8/sitepackages/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/sage9.6.beta2/local/var/lib/sage/venvpython3.8/lib/python3.8/sitepackages/sage/misc/sage_unittest.py", line 297, in run test_method(tester=tester) File "/home/roed/sage9.6.beta2/local/var/lib/sage/venvpython3.8/lib/python3.8/sitepackages/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/sage9.6.beta2/local/var/lib/sage/venvpython3.8/lib/python3.8/sitepackages/sage/misc/sage_unittest.py", line 297, in run test_method(tester=tester) File "/home/roed/sage9.6.beta2/local/var/lib/sage/venvpython3.8/lib/python3.8/sitepackages/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/sage9.6.beta2/local/var/lib/sage/venvpython3.8/lib/python3.8/sitepackages/sage/misc/sage_unittest.py", line 297, in run test_method(tester=tester) File "/home/roed/sage9.6.beta2/local/var/lib/sage/venvpython3.8/lib/python3.8/sitepackages/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 warnlong 40.9 randomseed=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 warnlong 40.9 randomseed=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 warnlong 40.9 randomseed=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 warnlong 40.9 randomseed=317472647872885548861559054668703098865 src/sage/matrix/matrix_cdv.pyx # 1 doctest failed sage t long warnlong 40.9 randomseed=317472647872885548861559054668703098865 src/sage/rings/laurent_series_ring.py # 8 doctests failed sage t long warnlong 40.9 randomseed=317472647872885548861559054668703098865 src/sage/rings/multi_power_series_ring_element.py # 1 doctest failed sage t long warnlong 40.9 randomseed=317472647872885548861559054668703098865 src/sage/categories/complete_discrete_valuation.py # 1 doctest failed sage t long warnlong 40.9 randomseed=317472647872885548861559054668703098865 src/sage/matrix/matrix2.pyx # 1 doctest failed
comment:37 Changed 9 months ago by
Reviewers:  Ben Hutz → Ben Hutz, David Roe 

Status:  needs_review → needs_work 
comment:38 Changed 9 months ago by
Commit:  6d7e43e8134c061b464c86cd7e085637456b61ce → 6a2d2c932cf557410b1632c99e34bc070d790080 

Branch pushed to git repo; I updated commit sha1. New commits:
6a2d2c9  Laurent series ring is a field

comment:39 Changed 9 months ago by
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
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.
comment:41 Changed 9 months ago by
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:43 Changed 9 months ago by
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
Commit:  6a2d2c932cf557410b1632c99e34bc070d790080 → dc51f10b50bf804dca35589ee9f86adfeebaa8f2 

Branch pushed to git repo; I updated commit sha1. New commits:
dc51f10  remove failing doctest

comment:45 Changed 9 months ago by
Status:  needs_work → needs_review 

comment:46 Changed 9 months ago by
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
Commit:  dc51f10b50bf804dca35589ee9f86adfeebaa8f2 → 57a41b22f7375eec096c2af524ba5b7c871ed58c 

Branch pushed to git repo; I updated commit sha1. New commits:
57a41b2  small fix

comment:48 Changed 7 months ago by
Milestone:  sage8.1 → sage9.7 

Status:  needs_review → needs_work 
red branch => needs work
comment:49 Changed 6 months ago by
Commit:  57a41b22f7375eec096c2af524ba5b7c871ed58c → bd75b0dcfc6b0f450463cfa0103184a73fac1ffa 

Branch pushed to git repo; I updated commit sha1. New commits:
bd75b0d  Merge branch 'develop' into echelon_form

comment:50 Changed 6 months ago by
Status:  needs_work → needs_review 

I fixed merging issues.
I put needs review in order to get feedback from the patchbot.
comment:51 Changed 6 months ago by
Commit:  bd75b0dcfc6b0f450463cfa0103184a73fac1ffa → 27b57b89b3b2b2962ead3a8b96468d5c0d943e82 

Branch pushed to git repo; I updated commit sha1. New commits:
27b57b8  fix pyflakes

comment:52 Changed 6 months ago by
Branch:  u/caruso/padic_hermite → u/saraedum/padic_hermite 

comment:53 Changed 6 months ago by
Commit:  27b57b89b3b2b2962ead3a8b96468d5c0d943e82 → ecb4f01b734724d7f31e578a23fa374a08115ceb 

Branch pushed to git repo; I updated commit sha1. New commits:
ecb4f01  Deduplicate code for complete discrete valuation rings and fields

comment:54 Changed 6 months ago by
Commit:  ecb4f01b734724d7f31e578a23fa374a08115ceb → 3c4cfd951811903df59b93ce7359ae6dacf3f934 

comment:55 Changed 6 months ago by
Commit:  3c4cfd951811903df59b93ce7359ae6dacf3f934 → cb2404be69851ea823d1572504fd4001beafec64 

comment:56 Changed 6 months ago by
Are you using any results from the articles you wrote on padic precision? If so, I think that it would be nice to reference the articles in the ALGORITHM sections.
New commits:
eb97013  Improve documentation of CompleteDiscreteValuationRings.ParentMethods

ecb4f01  Deduplicate code for complete discrete valuation rings and fields

9ec4bcf  Fix handling of transformation=false in echelonize

3c4cfd9  Drop unused code

2638908  Fix LaTeX escaping in docstrings

0d8c61f  Improve docstrings

941f1c0  Drop commented out definitions

cb2404b  Remove typo

comment:57 Changed 6 months ago by
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
Commit:  cb2404be69851ea823d1572504fd4001beafec64 → ab32baca7f7aeae441a139fa4e1f1ed666ba5409 

comment:59 Changed 6 months ago by
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:
f595901  Cleanup implementation of matrix_cdv

ab32bac  Restore removed doctests

comment:60 Changed 6 months ago by
Branch:  u/saraedum/padic_hermite → u/caruso/padic_hermite 

comment:61 Changed 6 months ago by
Commit:  ab32baca7f7aeae441a139fa4e1f1ed666ba5409 → 1068baa478b35940165b106a2c7d361256d88d85 

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:
1068baa  fix small bug

comment:62 Changed 6 months ago by
I'm getting the following warning when Sage starts:
/home/xcaruso/sage/local/var/lib/sage/venvpython3.8/lib/python3.8/sitepackages/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
Commit:  1068baa478b35940165b106a2c7d361256d88d85 → 2094339c2fcd4f26de9056193f199431c09b40f9 

Branch pushed to git repo; I updated commit sha1. New commits:
2094339  Revert "Deduplicate code for complete discrete valuation rings and fields"

comment:64 Changed 6 months ago by
Commit:  2094339c2fcd4f26de9056193f199431c09b40f9 → 50520b68c430047dad8c3702c0232c4531658ad8 

Branch pushed to git repo; I updated commit sha1. New commits:
50520b6  do not use get_unsafe in py files

comment:65 Changed 6 months ago by
Branch:  u/caruso/padic_hermite → u/saraedum/padic_hermite 

comment:66 Changed 6 months ago by
Commit:  50520b68c430047dad8c3702c0232c4531658ad8 → e6e1606e281dc6ec165c88c784a63e1aee20fafa 

Branch pushed to git repo; I updated commit sha1. New commits:
e6e1606  Fix syntax error

comment:67 Changed 6 months ago by
Branch:  u/saraedum/padic_hermite → u/caruso/padic_hermite 

comment:68 Changed 4 months ago by
Commit:  e6e1606e281dc6ec165c88c784a63e1aee20fafa → 26f0fd223edb143de3465f454e2dde5be1c06f9c 

Status:  needs_review → needs_work 
comment:69 Changed 2 months ago by
Milestone:  sage9.7 → sage9.8 

New commits:
Smith form of a padic matrix
Code moved into the category
Factorisation of code
Small fix
Move relevant code to sage.matrix.matrix_cdv_dense
Fix import
Add method tracks_precision()
Echelon form for matrices over CDVR/CDVF