Opened 6 years ago

Closed 6 years ago

#17501 closed defect (fixed)

Add is_unique_factorization_domain to IntegerMod rings and Polynomial Rings

Reported by: bruno Owned by:
Priority: major Milestone: sage-6.5
Component: commutative algebra Keywords: UFD
Cc: Merged in:
Authors: Bruno Grenet Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 4162112 (Commits) Commit: 4162112539daee466e3c95b44cb142a27cb3068e
Dependencies: Stopgaps:

Description (last modified by bruno)

Right now, there is no Zmod(k).is_unique_factorization_domain() method, while one can test these rings for being field, integral domain, ... The same holds for polynomial rings.

This is motivated by ticket #15790 in which `is_unique_factorization_domain()' is used.

Change History (12)

comment:1 Changed 6 years ago by bruno

  • Component changed from finite rings to commutative algebra
  • Description modified (diff)
  • Summary changed from Add is_unique_factorization_domain to IntegerMod rings to Add is_unique_factorization_domain to IntegerMod rings and Polynomial Rings

comment:2 Changed 6 years ago by bruno

  • Description modified (diff)

comment:3 Changed 6 years ago by bruno

  • Branch set to u/bruno/add_is_unique_factorization_domain_to_integermod_rings

comment:4 Changed 6 years ago by bruno

  • Commit set to 02942925d0733b91bfc99a1e454e3ca40789d52a
  • Status changed from new to needs_review

New commits:

0294292Add is_unique_factorization_domain for integer_mod and polynomial rings

comment:5 follow-up: Changed 6 years ago by tscrim

Since this checks for the prime, which is the same test for is_field, IMO we should just call is_field (and do the category refinement too).

comment:6 in reply to: ↑ 5 Changed 6 years ago by bruno

Replying to tscrim:

Since this checks for the prime, which is the same test for is_field, IMO we should just call is_field (and do the category refinement too).

Seems reasonable indeed. I do not understand the "category refinement" part of your comment!

comment:7 follow-up: Changed 6 years ago by tscrim

  • Reviewers set to Travis Scrimshaw

Zmod is in the category of Rings by default because we don't want to do the prime check. However, if we do do the check, then we want Zmod to be in the category of Fields if applicable. You can see this by:

sage: R = Zmod(5)
sage: R.category()
Join of Category of finite commutative rings and Category of subquotients of monoids and Category of quotients of semigroups
sage: R.is_field()
True
sage: R.category()
Join of Category of finite fields and Category of subquotients of monoids and Category of quotients of semigroups

Moreover you get extra methods and/or shortcuts (like lcm on elements).

Do you mind making the change Bruno?

comment:8 in reply to: ↑ 7 Changed 6 years ago by bruno

I looked into sage.rings.finite_rings.integer_mod_ring.py but was unable to find how the change of category occurs. In particular, I looked into the is_field() method but nothing seems to be done there.

comment:9 Changed 6 years ago by tscrim

It's this part of the sage.rings.finite_rings.integer_mod_ring.is_field method:

is_prime = self.order().is_prime(proof=proof)
if is_prime:
    self._refine_category_(Fields())
    self._factory_data[3]['category'] = Fields()

comment:10 Changed 6 years ago by tscrim

  • Branch changed from u/bruno/add_is_unique_factorization_domain_to_integermod_rings to public/commutatative_algebra/add_is_ufd-17501
  • Commit changed from 02942925d0733b91bfc99a1e454e3ca40789d52a to 4162112539daee466e3c95b44cb142a27cb3068e

I've made the change. I also changed the default input to proof=None to reflect that of is_field (at the end of the day, I believe the is_prime uses a global flag of some kind to make the default choice). If you're happy with my changes, then you can set this to a positive review.


New commits:

4162112Change UFD check to call is_field.

comment:11 Changed 6 years ago by bruno

  • Status changed from needs_review to positive_review

Of course, it was quite obvious... Thank you for the change! LGTM, positive review.

comment:12 Changed 6 years ago by vbraun

  • Branch changed from public/commutatative_algebra/add_is_ufd-17501 to 4162112539daee466e3c95b44cb142a27cb3068e
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.