Opened 5 years ago

Closed 5 years ago

#23920 closed enhancement (fixed)

py3: richcmp for ideals of multivariate polynomials

Reported by: chapoton Owned by:
Priority: major Milestone: sage-8.1
Component: python3 Keywords:
Cc: tscrim, jdemeyer, jhpalmieri, aapitzsch, fbissey Merged in:
Authors: Frédéric Chapoton Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: bb784e1 (Commits, GitHub, GitLab) Commit: bb784e1de045013aaa34c7bebcc6628dc1138a79
Dependencies: Stopgaps:

Status badges

Description

split off from #23787

refactoring of the comparison of ideals in multivariate polynomials

  • to get rid of cmp
  • consistent comparison by containment
  • when a total order is needed, compare instead by gens

Change History (13)

comment:1 Changed 5 years ago by chapoton

  • Branch set to u/chapoton/23920
  • Commit set to f077b000343f55e0cf9e64c098c8fa9bbabcc5af

New commits:

f077b00trac 23920 richcmp for ideals of multi-polynomials

comment:2 Changed 5 years ago by git

  • Commit changed from f077b000343f55e0cf9e64c098c8fa9bbabcc5af to b5f4dd4db2466addc4d88b8c5ba5710e467e4660

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

b5f4dd4trac 23920 fixing some doctests (order changed)

comment:3 Changed 5 years ago by git

  • Commit changed from b5f4dd4db2466addc4d88b8c5ba5710e467e4660 to 4b74c315a297a6af87c7d995eed7af9d995d262b

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

4b74c31trac 23920 chasing triggered bugs all around the place

comment:4 Changed 5 years ago by git

  • Commit changed from 4b74c315a297a6af87c7d995eed7af9d995d262b to 7f8200b042b38804cd628a4151cd2592c23eeaa3

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

7f8200btrac 23920 fix last doctest failing + py3 warning

comment:5 Changed 5 years ago by chapoton

  • Cc tscrim jdemeyer jhpalmieri aapitzsch fbissey added
  • Status changed from new to needs_review

This should be ready to go. This refactors the comparison of ideals in multi-variate polynomial rings. Comparison is now meaning "for containment" in all cases.

Besides, it tries to avoid computing Grobner bases, by first comparing the generators.

Please review.

comment:6 Changed 5 years ago by chapoton

green bot!

comment:7 Changed 5 years ago by chapoton

now this is going to conflict with #23814

comment:8 Changed 5 years ago by chapoton

Could someone please review this? there should be no real conflict with #23814.

comment:9 Changed 5 years ago by tscrim

Rather than comparing gens as a list, I would compare them as sets. That way, reordering of gens doesn't require a GB computation, e.g.:

sage: R.<x,y> = ZZ[]
sage: I = R.ideal([x^2, x*y - y^2 + 2])
sage: Ip = R.ideal([x*y - y^2 + 2, x^2])
sage: I.gens()
[x^2, x*y - y^2 + 2]
sage: Ip.gens()
[x*y - y^2 + 2, x^2]
sage: I == Ip   # Avoid GB here
True

Also, what is the rationale for this change:

@@ -532,8 +529,13 @@ class SchemeMorphism_polynomial_affine_space(SchemeMorphism_polynomial):
 
         #homogenize the domain and codomain
         A = self.domain().projective_embedding(ind[0]).codomain()
-        B = self.codomain().projective_embedding(ind[1]).codomain()
-        H = Hom(A, B)
+        if self.is_endomorphism():
+            B = A
+            H = End(A)
+        else:
+            B = self.codomain().projective_embedding(ind[1]).codomain()
+            H = Hom(A, B)
+
         newvar = A.ambient_space().coordinate_ring().gen(ind[0])
 
         N = A.ambient_space().dimension_relative()

comment:10 Changed 5 years ago by git

  • Commit changed from 7f8200b042b38804cd628a4151cd2592c23eeaa3 to bb784e1de045013aaa34c7bebcc6628dc1138a79

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

bb784e1trac 23920 using set(gens()) == set(gens())

comment:11 Changed 5 years ago by chapoton

  • I have modifed the comparison to use sets of gens.
  • The changes in scheme_morphisms and dynamical systems could probably be made independently now, even if at some point they were intertwined with the main issue here.
  • the precise change you talk about was made so that one could replace comparison of domains (in dynamical systems) with a call to is_endomorphism.

comment:12 Changed 5 years ago by tscrim

  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

Okay. LGTM. Thanks.

comment:13 Changed 5 years ago by vbraun

  • Branch changed from u/chapoton/23920 to bb784e1de045013aaa34c7bebcc6628dc1138a79
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.