Opened 16 months ago

Last modified 16 months ago

#25836 new defect

in PrincipalIdealDomains() and in UniqueFactorizationDomains() give wrong answers

Reported by: ababei Owned by:
Priority: major Milestone: sage-8.3
Component: commutative algebra Keywords: Days95
Cc: vdelecroix Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

One of the tests in src/sage/categories/principal_ideal_domains.py (lines 80-83) is testing that the following ring is not a UFD

sage: K = QuadraticField(5)
sage: O = K.maximal_order()
sage: O in UniqueFactorizationDomains()
False

However, trying rings of integers of number fields, which are UFDs or PIDs still gives false as an answer.

sage: K = QuadraticField(-1)
sage: O = K.maximal_order()
sage: O in UniqueFactorizationDomains()
False
sage: O in PrincipalIdealDomains()
False

Change History (1)

comment:1 Changed 16 months ago by nbruin

That's an interesting example, since the integer ring of Q(\sqrt(5)) is actually a UFD and a PID.

Looking at the code of UniqueFactorizationDomains().__contains__ one can see that any non-trivial functionality should go into O.is_unique_factorization_domain(). In this case the implementation could be return self.class_number() == 1. However, such a method isn't there, so you're seeing a true/false based on the stored categorical info, and integer rings are not categorically UFDs.

There may be bad ramifications from answering UniqueFactorizationDomains().__contains__ via a probably very expensive class group computation, so there might be good practical reasons why a more functional is_unique_factorization_domain method isn't provided.

In any case, it may be worth changing the example to K=QuadraticField(-5) or so. It looks a bit silly to use a mathematically UFD in a setting to illustrate non-UFD behaviour.

I also don't see how PolynomialRing(O,'x') can be a PID: the ideal (7,x) is really not principal. Were these doctests written by a non-mathematician?

Last edited 16 months ago by nbruin (previous) (diff)
Note: See TracTickets for help on using tickets.