Opened 11 years ago
Closed 7 years ago
#10513 closed defect (fixed)
Coercion and category framework for modules
Reported by:  SimonKing  Owned by:  robertwb 

Priority:  major  Milestone:  sage6.6 
Component:  coercion  Keywords:  coercion, category framework, modules 
Cc:  vbraun, novoselt, jpflori  Merged in:  
Authors:  Simon King, Peter Bruin  Reviewers:  JeanPierre Flori, Vincent Delecroix 
Report Upstream:  N/A  Work issues:  
Branch:  9865576 (Commits, GitHub, GitLab)  Commit:  9865576ec5600a1502e5c5e455e5f2ce08406f4c 
Dependencies:  #16507, #17578, #17561, #18040, #17562  Stopgaps: 
Description (last modified by )
Currently, modules in Sage are based on the old coercion model and do not cooperate well with the category framework. This ticket updates almost all module classes to the use new coercion model and the category framework.
Moreover, some coercions go wrong:
sage: V3 = QQ^3 sage: M = V3 / [[1,2,3]] sage: V2 = QQ^2 sage: N = V3 / [[1,1,1]] # The quotient maps are of course coercions sage: f3M = M.coerce_map_from(V3) sage: f3N = N.coerce_map_from(V3) # The following is a bug... sage: V2.has_coerce_map_from(N) True sage: M.has_coerce_map_from(V2) True sage: N.has_coerce_map_from(V2) True #... because the composition of coercion # from N to V2 to M should be a coercion, # but it isn't: sage: M.has_coerce_map_from(N) False # Moreover, we obtain a noncommuting triangle of # coercions: sage: fM2 = V2.coerce_map_from(M) sage: fN2 = V2.coerce_map_from(N) sage: [fM2(f3M(x)) for x in V3.basis()] [(1, 0), (0, 1), (1/3, 2/3)] sage: [fN2(f3N(x)) for x in V3.basis()] [(1, 0), (0, 1), (1, 1)]
Comparison goes wrong as well:
sage: V2==M True sage: M==V2 False
In the above examples, this ticket removes the coercion between abstract module V^2
and the quotient modules, and makes sure that V2!=M
.
Attachments (1)
Change History (69)
Changed 11 years ago by
comment:1 Changed 11 years ago by
 Status changed from new to needs_review
 Type changed from PLEASE CHANGE to defect
comment:2 followup: ↓ 3 Changed 11 years ago by
 Status changed from needs_review to needs_info
One of the things that I did in #9713 was to replace sage.structure.parent_gens.ParentWithAdditiveAbelianGens
with sage.structure.parent.Parent
as the base class of Module
. At the top of sage/structure/parent_gens.pyx
it says that
This class is being deprecated, see ``sage.structure.parent.Parent`` and ``sage.structure.category_object.CategoryObject`` for the new model.
But your patch does not change the Module base class. So I'm a bit confused about what the right way of doing things is.
comment:3 in reply to: ↑ 2 Changed 11 years ago by
Replying to vbraun:
At the top of
sage/structure/parent_gens.pyx
it says thatThis class is being deprecated, see ``sage.structure.parent.Parent`` and ``sage.structure.category_object.CategoryObject`` for the new model.But your patch does not change the Module base class.
Thank you, I simply wasn't aware of the deprecation. Changing it now, and if tests pass I'll update my patch.
comment:4 Changed 11 years ago by
 Status changed from needs_info to needs_work
I just noticed that ParentWithBase
is requested in various other files. So, it will take a while until I can replace ParentWithAdditiveAbelianGens
by Parent
.
comment:5 Changed 11 years ago by
 Work issues set to replace ParentWithAdditiveGens by Parent; arithmetic for module elements using single underscore methods
comment:6 Changed 8 years ago by
 Milestone changed from sage5.11 to sage5.12
comment:7 Changed 8 years ago by
 Milestone changed from sage6.1 to sage6.2
comment:8 Changed 8 years ago by
 Milestone changed from sage6.2 to sage6.3
comment:9 Changed 7 years ago by
 Dependencies set to #16507
I am trying to finish this ticket. I currently have a branch that passes all doctests, but I am still improving some of the code and adding documentation.
comment:10 followup: ↓ 25 Changed 7 years ago by
 Branch set to u/pbruin/10513coercion_and_categories_for_modules
 Commit set to ed13085009f4b79544e1aa9970820b7f1bf6cb8f
 Status changed from needs_work to needs_review
 Work issues replace ParentWithAdditiveGens by Parent; arithmetic for module elements using single underscore methods deleted
OK, ready for review. There is still some future work to be done:
 move the last few classes from
Module_old
toModule
. There are only four of these remaining:sage.coding.linear_code.LinearCode
sage.modular.abvar.finite_subgroup.FiniteSubgroup
sage.modular.overconvergent.genus0.OverconvergentModularFormsSpace
sage.structure.formal_sum.FormalSums
 make
sage.modules.matrix_morphism.MatrixMorphism_abstract
use singleunderscore methods for addition, subtraction, scalar multiplication and composition.
I think it is better to do these in separate tickets.
comment:11 Changed 7 years ago by
sage.coding.linear_code.LinearCode
is taken care of in #16644.
comment:12 Changed 7 years ago by
 Milestone changed from sage6.3 to sage6.4
comment:13 Changed 7 years ago by
 Commit changed from ed13085009f4b79544e1aa9970820b7f1bf6cb8f to 5126b66a6c5ba2d52b6daff6ee489aa8b48a7548
Branch pushed to git repo; I updated commit sha1. New commits:
5126b66  Trac 10513: remove source line number in doctest

comment:14 Changed 7 years ago by
 Description modified (diff)
comment:15 Changed 7 years ago by
 Commit changed from 5126b66a6c5ba2d52b6daff6ee489aa8b48a7548 to ddda401f855fc34936e204eb14fa3d0af597b76f
Branch pushed to git repo; I updated commit sha1. New commits:
ddda401  Merge branch 'develop' into ticket/10513coercion_and_categories_for_modules

comment:16 Changed 7 years ago by
 Cc jpflori added
comment:17 Changed 7 years ago by
 Commit changed from ddda401f855fc34936e204eb14fa3d0af597b76f to ed6bbf1bf478d40fe467d2f56bc58b0ad482a840
comment:18 Changed 7 years ago by
Could you add a little documentation about the coercion_reverse
thing?
Or rather, move it in the actual code, rather than in comments?
Not that the category/coercion stuff is already obscure, but...
comment:19 Changed 7 years ago by
I guess my comment should have been made in #16507.
comment:20 Changed 7 years ago by
 Status changed from needs_review to needs_work
 Work issues set to fix pickling problem
After merging with 6.5.beta3 (which git
does automatically, despite Trac being unable to automerge), there is one failing doctest:
File "src/sage/modules/free_module.py", line 6074, in sage.modules.free_module.FreeModule_submodule_with_basis_field Failed example: TestSuite(W).run() Expected nothing Got: Failure in _test_pickling: Traceback (most recent call last): File "/home/bruinpj/src/sage/local/lib/python2.7/sitepackages/sage/misc/sage_unittest.py", line 282, in run test_method(tester = tester) File "sage/structure/sage_object.pyx", line 543, in sage.structure.sage_object.SageObject._test_pickling (build/cythonized/sage/structure/sage_object.c:4553) tester.assertEqual(loads(dumps(self)), self) File "/home/bruinpj/src/sage/local/lib/python/unittest/case.py", line 513, in assertEqual assertion_func(first, second, msg=msg) File "/home/bruinpj/src/sage/local/lib/python/unittest/case.py", line 506, in _baseAssertEqual raise self.failureException(msg) AssertionError: Vector space of degree 3 and dimension 1 over Fraction Field of Univariate Polynomial Ring in x over Rational Field User basis matrix: [1 1 x] != Vector space of degree 3 and dimension 1 over Fraction Field of Univariate Polynomial Ring in x over Rational Field User basis matrix: [1 1 x]  The following tests failed: _test_pickling
This is due to the cached echelonized_basis_matrix
behaving badly under pickling:
sage: K.<x> = FractionField(QQ['x']) sage: M = K^3 sage: W = M.span_of_basis([[1, 1, x]]) sage: W.echelonized_basis_matrix() [1 1 x] sage: s = dumps(W) sage: V = loads(s) sage: V.echelonized_basis_matrix() [1, 1, x] sage: type(_) <type 'list'>
There does not seem to be any such problem with basis_matrix()
:
sage: W.basis_matrix() [1 1 x] sage: V.basis_matrix() [1 1 x]
comment:21 Changed 7 years ago by
 Dependencies changed from #16507 to #16507, #17527
 Work issues fix pickling problem deleted
The pickling problem is independent of this ticket, see #17527.
comment:22 Changed 7 years ago by
 Commit changed from ed6bbf1bf478d40fe467d2f56bc58b0ad482a840 to 2f52a3cb9945479d4d7298c06119bde5481c6d27
comment:23 Changed 7 years ago by
 Dependencies changed from #16507, #17527 to #16507
 Status changed from needs_work to needs_review
The pickling problem can actually be circumvented in a more elegant way by avoiding an unnecessary caching step (the echelon form of a matrix is already cached).
comment:24 Changed 7 years ago by
 Commit changed from 2f52a3cb9945479d4d7298c06119bde5481c6d27 to 2d7f0759c12fbeeb45d42c2d6160f8d925a22252
Branch pushed to git repo; I updated commit sha1. New commits:
2d7f075  Trac 10513: small documentation improvement

comment:25 in reply to: ↑ 10 ; followup: ↓ 26 Changed 7 years ago by
I had a look at the work done here and it looks nice.
Replying to pbruin:
OK, ready for review. There is still some future work to be done:
 move the last few classes from
Module_old
toModule
. There are only four of these remaining:
sage.coding.linear_code.LinearCode
sage.modular.abvar.finite_subgroup.FiniteSubgroup
sage.modular.overconvergent.genus0.OverconvergentModularFormsSpace
sage.structure.formal_sum.FormalSums
 make
sage.modules.matrix_morphism.MatrixMorphism_abstract
use singleunderscore methods for addition, subtraction, scalar multiplication and composition.I think it is better to do these in separate tickets.
Did you open tickets for these?
And in a couple of changes to src/sage/geometry/cone.py
you modify code not to use _ in _
as ther is no coercion defined.
Any ticket opened to implement such a coercion?
comment:26 in reply to: ↑ 25 ; followup: ↓ 27 Changed 7 years ago by
Replying to jpflori:
I had a look at the work done here and it looks nice.
Thanks!
Replying to pbruin:
OK, ready for review. There is still some future work to be done:
 move the last few classes from
Module_old
toModule
. There are only four of these remaining:
sage.coding.linear_code.LinearCode
sage.modular.abvar.finite_subgroup.FiniteSubgroup
sage.modular.overconvergent.genus0.OverconvergentModularFormsSpace
sage.structure.formal_sum.FormalSums
 make
sage.modules.matrix_morphism.MatrixMorphism_abstract
use singleunderscore methods for addition, subtraction, scalar multiplication and composition.I think it is better to do these in separate tickets.
Did you open tickets for these?
I just created #17543 and #17544. (See comment:11 for LinearCode
.)
And in a couple of changes to
src/sage/geometry/cone.py
you modify code not to use_ in _
as ther is no coercion defined. Any ticket opened to implement such a coercion?
I'm not sure if this is desired; the absence of these particular maps may have been an (unintended) consequence of implementing the new coercion model in the simplest possible way, but it may have been intentional as well, I don't remember.
comment:27 in reply to: ↑ 26 ; followup: ↓ 28 Changed 7 years ago by
Replying to pbruin:> > Any ticket opened to implement such a coercion?
I'm not sure if this is desired; the absence of these particular maps may have been an (unintended) consequence of implementing the new coercion model in the simplest possible way, but it may have been intentional as well, I don't remember.
I was asking because it used to be possible to use the in
keyword when the classes were using the old coercion model.
comment:28 in reply to: ↑ 27 Changed 7 years ago by
Replying to jpflori:
Replying to pbruin:> > Any ticket opened to implement such a coercion?
I'm not sure if this is desired; the absence of these particular maps may have been an (unintended) consequence of implementing the new coercion model in the simplest possible way, but it may have been intentional as well, I don't remember.
I was asking because it used to be possible to use the
in
keyword when the classes were using the old coercion model.
After looking at this again, I think the reason was as follows: a cone C is a certain type of subset of a vector space V, and C contains a largest maximal linear subspace S. Now to test if an element x of C is in S, the coercion framework checks if S(x) == x, but when trying to comparing these two elements it cannot find a common parent for the toric lattice and the linear subspace. This seems to be because of the following method in ToricLattice_generic
:
# We need to override this function, otherwise e.g. the sum of elements of # different lattices of the same dimension will live in ZZ^n. def construction(self): r""" Return the functorial construction of ``self``. OUTPUT:  ``None``, we do not think of toric lattices as constructed from simpler objects since we do not want to perform arithmetic involving different lattices. [...] """ return None
The reason why it used to work with the old coercion model might have been a peculiarity of some _coerce_impl()
method. Trying to fix this (so that we can use ... in ...
again) will probably lead to all kind of other problems, so I'd prefer not to do that on this ticket.
comment:29 Changed 7 years ago by
 Reviewers set to JeanPierre Flori
 Status changed from needs_review to positive_review
Ok, I buy your explanation, thanks for digging this out.
comment:30 followup: ↓ 32 Changed 7 years ago by
I get lots of doctest failures all originating at something like
TypeError: must be intialized with a type, not Manin Symbol List of weight 4 for Gamma0(11)
comment:31 Changed 7 years ago by
 Status changed from positive_review to needs_work
comment:32 in reply to: ↑ 30 Changed 7 years ago by
Replying to vbraun:
I get lots of doctest failures all originating at something like
TypeError: must be intialized with a type, not Manin Symbol List of weight 4 for Gamma0(11)
Can you give more details and/or a link to a log file? I am not getting any doctest failures in sage/modular
(which I would guess is where these failures occur) after merging this branch with 6.5.beta4, both on x86_64 and on ARM.
comment:33 followup: ↓ 36 Changed 7 years ago by
I think it's because of #10962.
comment:34 followup: ↓ 35 Changed 7 years ago by
Does this patch address anything of https://groups.google.com/forum/#!topic/sagedevel/7n5xtT9Dw2g by chance?
comment:35 in reply to: ↑ 34 Changed 7 years ago by
Replying to jdemeyer:
Does this patch address anything of https://groups.google.com/forum/#!topic/sagedevel/7n5xtT9Dw2g by chance?
No, I don't think so. (It does solve #17576, though.)
comment:36 in reply to: ↑ 33 Changed 7 years ago by
comment:37 Changed 7 years ago by
 Dependencies changed from #16507 to #16507, #17578
 Status changed from needs_work to positive_review
comment:38 followup: ↓ 41 Changed 7 years ago by
 Status changed from positive_review to needs_info
In src/sage/modules/free_module_element.pyx
, you changed in one place
except TypeError:
to
except (TypeError, ValueError):
Is this change needed? If yes, it should also be applied to the other place with similar code (there is one __init__
method for dense and one for sparse vectors).
comment:39 Changed 7 years ago by
(I'm asking because this is a conflict with #17561)
comment:40 Changed 7 years ago by
 Commit changed from 2d7f0759c12fbeeb45d42c2d6160f8d925a22252 to 99ec4cc13099658e0bc762a6e8d230c3956c7150
Branch pushed to git repo; I updated commit sha1. New commits:
99ec4cc  Trac 10513: revert a change in try...except

comment:41 in reply to: ↑ 38 Changed 7 years ago by
Replying to jdemeyer:
In
src/sage/modules/free_module_element.pyx
, you changed in one placeexcept TypeError:to
except (TypeError, ValueError):Is this change needed?
Apparently not; the last commit reverts it.
comment:42 Changed 7 years ago by
 Status changed from needs_info to positive_review
comment:43 Changed 7 years ago by
 Dependencies changed from #16507, #17578 to #16507, #17578, #17561
 Status changed from positive_review to needs_work
There is a minor conflict with #17561.
comment:44 Changed 7 years ago by
 Branch changed from u/pbruin/10513coercion_and_categories_for_modules to u/jdemeyer/ticket/10513
 Modified changed from 01/11/15 21:33:50 to 01/11/15 21:33:50
comment:45 Changed 7 years ago by
 Commit changed from 99ec4cc13099658e0bc762a6e8d230c3956c7150 to b1287e75962a2dc590f1fa22acc90a4e53383aab
 Milestone changed from sage6.4 to sagepending
 Status changed from needs_work to positive_review
comment:46 Changed 7 years ago by
 Status changed from positive_review to needs_work
On 6.5.beta5:
sage: %time for p in prime_range(10^5): V=(GF(p))^3 CPU times: user 3.63 s, sys: 28 ms, total: 3.66 s Wall time: 3.65 s
With this branch:
sage: %time for p in prime_range(10^5): V=(GF(p))^3 CPU times: user 13.4 s, sys: 29 ms, total: 13.5 s Wall time: 13.5 s
I think that's a significant regression in performance. It's probably due to the construction of the categorywithbase. If you change the category to be "Category of vector spaces over Fields" or "over Finite Fields" you'll get much better performance, because there's not a new category to be constructed for every single space.
(this stuff isn't academicsuch spaces really get made a lot in CRT code). See also #17360, where the issue causes a memleak (that seems to be fine here). See also http://trac.sagemath.org/ticket/10963#comment:506 where this issue was mentioned right at the start of the development of these parametrized categories.
comment:47 followup: ↓ 48 Changed 7 years ago by
Yep, with

src/sage/modules/free_module.py
diff git a/src/sage/modules/free_module.py b/src/sage/modules/free_module.py index 4cf68fc..dc9248c 100644
a b done from the right side.""") 721 721 722 722 if category is None: 723 723 from sage.categories.all import Fields, FreeModules, VectorSpaces 724 if base_ring in Fields(): 725 category = VectorSpaces(base_ring) 724 F=Fields() 725 if base_ring in F: 726 category = VectorSpaces(F) 726 727 else: 727 category = FreeModules(base_ring )728 category = FreeModules(base_ring.category()) 728 729 try: 729 730 if base_ring.is_finite() or rank == 0: 730 731 # Put the module in the category of finite enumerated
I get
sage: %time for p in prime_range(10^5): V=(GF(p))^3 CPU times: user 2.52 s, sys: 27 ms, total: 2.54 s Wall time: 2.53 s
In profiling this example I'm still finding
28774 0.054 0.000 0.329 0.000 category.py:2281(join)
so it seems we're still spending more than 10% of our time on dynamically constructing a category (taking the join with FiniteEnumeratedSets()
) just to get a list method. This looks like another candidate to improve (a bit).
On the other hand, part of this might by due to GF(p)
construction by itself. If I first do L=[GF(p) for p in prime_range(10^5)]
the run time drops to 1.19 s, there are less calls to join (exactly one per constructed field, so GF(p)
seems to need two category joins) and the impact of category.join
drops to 0.086
, which is about 67% of the run time then.
comment:48 in reply to: ↑ 47 ; followup: ↓ 50 Changed 7 years ago by
Replying to nbruin:
On the other hand, part of this might by due to
GF(p)
construction by itself. If I first doL=[GF(p) for p in prime_range(10^5)]
the run time drops to 1.19 s, there are less calls to join (exactly one per constructed field, soGF(p)
seems to need two category joins) and the impact ofcategory.join
drops to0.086
, which is about 67% of the run time then.
Indeed, the category of GF(p)
is a join of three categories:
sage: GF(3).category() Join of Category of finite fields and Category of subquotients of monoids and Category of quotients of semigroups
Maybe parts of the category code (like join
) should be transformed into Cython to make it faster?
comment:49 Changed 7 years ago by
 Branch changed from u/jdemeyer/ticket/10513 to u/pbruin/10513coercion_and_categories_for_modules
 Commit changed from b1287e75962a2dc590f1fa22acc90a4e53383aab to 6b63026ceea5e9daaee46ca821a85e4edd6ded8b
 Dependencies changed from #16507, #17578, #17561 to #16507, #17578, #17561, #11126
With the new commits, the category of free modules is FreeModules(base_ring.category())
by default, and instead of taking the join with FiniteEnumeratedSets
(if applicable), we add a list()
method.
There are two doctest failures:
sage t long warnlong 43.7 src/sage/matrix/matrix2.pyx # 1 doctest failed sage t long warnlong 43.7 src/sage/modules/free_module.py # 1 doctest failed
The first is caused by #11126; the second is a NotImplementedError
in repr()
caused by the fact that basis()
is not implemented for abstract free modules.
New commits:
f0957f0  Trac 10513: use FreeModules(base_ring.category()) instead of FreeModules(base_ring)

6b63026  Trac 10513: method FreeModule_generic.list() to replace FiniteEnumeratedSets magic

comment:50 in reply to: ↑ 48 Changed 7 years ago by
Replying to pbruin:
Indeed, the category of
GF(p)
is a join of three categories:sage: GF(3).category() Join of Category of finite fields and Category of subquotients of monoids and Category of quotients of semigroupsMaybe parts of the category code (like
join
) should be transformed into Cython to make it faster?
That's one option, but ultimately taking joins is a a rather dynamic operation that needs to investigate quite a bit of the category graph in order to ensure it's safe to use what's cached (can the result of a join depend on changes elsewhere in the graph?). In the case of finite fields, there is *no* variation in what the category is: this thing can be created up front (to accommodate GF(2), which is always present if I'm not mistaken), and just be set to be the category. The current problem is that the joining of categories happens in some of the more generic construction routines. We'd need a way of signalling that we want to shortcut that (a precise_category keyword that percolates up perhaps?). That's an optimization strategy that may have more payoff for this particular scenario than making "join" faster (which might still be worthwhile to do). In any case, that's for another ticket. Also: premature optimization etc. Thanks for looking into this.
comment:51 Changed 7 years ago by
 Commit changed from 6b63026ceea5e9daaee46ca821a85e4edd6ded8b to ac7098684fe07997da64bacee522d88c5530c38a
Branch pushed to git repo; I updated commit sha1. New commits:
6d5aaa1  Merge branch 'develop' into ticket/10513coercion_and_categories_for_modules

d1db7ea  Trac 10513: construct coerce_map as default conversion map instead of matrix morphism

7b09742  Trac 10513: checking if element_class is a method is no longer necessary

ac685ec  Trac 10513: update doctests

ac70986  Trac 10513: small documentation improvement

comment:52 Changed 7 years ago by
 Status changed from needs_work to needs_review
Everything should work again after this update.
comment:53 followup: ↓ 54 Changed 7 years ago by
 Status changed from needs_review to needs_work
Not everything...
some tests failed: sage t long src/sage/matrix/matrix2.pyx # 1 doctest failed sage t long src/sage/modular/modform/constructor.py # 1 doctest failed sage t long src/sage/modular/modform/eisenstein_submodule.py # 1 doctest failed sage t long src/sage/modular/modform/ambient_g1.py # 1 doctest failed sage t long src/sage/modular/arithgroup/congroup_gamma0.py # 1 doctest failed
comment:54 in reply to: ↑ 53 ; followup: ↓ 55 Changed 7 years ago by
Replying to vdelecroix:
Not everything...
some tests failed: sage t long src/sage/matrix/matrix2.pyx # 1 doctest failed sage t long src/sage/modular/modform/constructor.py # 1 doctest failed sage t long src/sage/modular/modform/eisenstein_submodule.py # 1 doctest failed sage t long src/sage/modular/modform/ambient_g1.py # 1 doctest failed sage t long src/sage/modular/arithgroup/congroup_gamma0.py # 1 doctest failed
I just tested again with 6.6.rc0 and #11126 merged, and got no failures. What kind of errors are you getting?
comment:55 in reply to: ↑ 54 Changed 7 years ago by
Replying to pbruin:
Replying to vdelecroix:
...
I just tested again with 6.6.rc0 and #11126 merged, and got no failures. What kind of errors are you getting?
Strange... I am launching the tests again.
comment:56 followup: ↓ 60 Changed 7 years ago by
comment:57 followup: ↓ 58 Changed 7 years ago by
Results of the new test... most of the failures disappeared except one
sage t long src/sage/matrix/matrix2.pyx ********************************************************************** File "src/sage/matrix/matrix2.pyx", line 1931, in sage.matrix.matrix2.Matrix.minpoly Failed example: m.minimal_polynomial() Exception raised: Traceback (most recent call last): ... NotImplementedError: is_squarefree() is only implemented for polynomials over principal ideal domains **********************************************************************
comment:58 in reply to: ↑ 57 Changed 7 years ago by
Replying to vdelecroix:
Results of the new test... most of the failures disappeared except one
sage t long src/sage/matrix/matrix2.pyx ********************************************************************** File "src/sage/matrix/matrix2.pyx", line 1931, in sage.matrix.matrix2.Matrix.minpoly Failed example: m.minimal_polynomial() Exception raised: Traceback (most recent call last): ... NotImplementedError: is_squarefree() is only implemented for polynomials over principal ideal domains **********************************************************************
This one is fixed by #11126. (The other failures were probably fixed by #17578 which is in 6.6.rc0.) I deliberately did not merge #11126 into this branch since I expected some discussion about it. Maybe this particular failure can be fixed in a more minimalistic way than as part of #11126.
comment:59 Changed 7 years ago by
 Dependencies changed from #16507, #17578, #17561, #11126 to #16507, #17578, #17561, #18040
comment:60 in reply to: ↑ 56 Changed 7 years ago by
 Milestone changed from sagepending to sage6.6
 Status changed from needs_work to needs_review
comment:61 Changed 7 years ago by
 Dependencies changed from #16507, #17578, #17561, #18040 to #16507, #17578, #17561, #18040, #17562
Is this then ready to go back to positive review? Vincent, did/could you run through the tests again?
comment:62 Changed 7 years ago by
comment:63 Changed 7 years ago by
At least on my computer
All tests passed!
comment:64 Changed 7 years ago by
 Reviewers changed from JeanPierre Flori to JeanPierre Flori, Vincent Delecroix
 Status changed from needs_review to positive_review
Thanks Vincent. I'm going to set this (back) to positive review.
comment:65 Changed 7 years ago by
 Status changed from positive_review to needs_work
Merge conflict with 6.7.beta0
comment:66 Changed 7 years ago by
 Commit changed from ac7098684fe07997da64bacee522d88c5530c38a to 9865576ec5600a1502e5c5e455e5f2ce08406f4c
Branch pushed to git repo; I updated commit sha1. New commits:
9865576  Merge branch 'develop' into ticket/10513coercion_and_categories_for_modules

comment:67 Changed 7 years ago by
 Status changed from needs_work to positive_review
There was only a minor conflict; I assume this doesn't need another review.
comment:68 Changed 7 years ago by
 Branch changed from u/pbruin/10513coercion_and_categories_for_modules to 9865576ec5600a1502e5c5e455e5f2ce08406f4c
 Resolution set to fixed
 Status changed from positive_review to closed
Implement coercion and category framework for free modules, submodules, quotient modules. Depends on #10496