#28862 closed enhancement (fixed)

Localization of integral domains

Reported by: soehms Owned by:
Priority: major Milestone: sage-9.1
Component: commutative algebra Keywords: rings integral domains localization fraction field
Cc: tscrim, vdelecroix Merged in:
Authors: Sebastian Oehms Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 44f816d (Commits, GitHub, GitLab) Commit: 44f816ddf5aee524df452434b9e6ffcee72a17bf
Dependencies: Stopgaps:

Status badges

Description

Localization is an important ring construction tool, which isn't available in Sage, so far. Whenever you have to extend a given integral domain such that it contains the inverses of a finite set of elements but should allow non injective homomorphic images this construction will be needed. Such situations occur for base rings of certain algebras, for example Hecke algebras as pointed out in comment 9 of ticket #27371.

This ticket implements a class for this construction following an example given in the reference pages on coercion (thanks to Vincent for this hint): coercion example

Change History (15)

comment:1 Changed 19 months ago by soehms

  • Branch set to u/soehms/localization

comment:2 Changed 19 months ago by soehms

  • Commit set to 8065d4d01dbad333b0726084f35815281891b67c
  • Status changed from new to needs_review

The first example I've included in my initial attempt raises the following questions:

How should the behavior of matrix inversion over a localization be? By default it is a matrix over the field of fraction. In the doc-string of sage.matrix.matrix0.__invert__ this is explained by analogy to the behavior over the integers: parent(~1) == Rational Field. I think for a matrix over a localization the user would expect to obtain a matrix over an appropriate localization again (being equal or extending the former one). Shouldn't we change this behavior, as well?


New commits:

8065d4d28862: initial version

comment:3 Changed 19 months ago by git

  • Commit changed from 8065d4d01dbad333b0726084f35815281891b67c to 96da2b04862bfac29b4fd777e03f1a63eb3f1450

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

96da2b028862: fix pyflakes hints

comment:4 Changed 19 months ago by git

  • Commit changed from 96da2b04862bfac29b4fd777e03f1a63eb3f1450 to 804a62cea35badee839342e57a139742a28a06a4

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

804a62c28862: doctest syntax

comment:5 Changed 19 months ago by git

  • Commit changed from 804a62cea35badee839342e57a139742a28a06a4 to 35e87fcbf81e656137fd13983a9ce3630364e405

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

35e87fc28862: typos and some cleaning

comment:6 Changed 17 months ago by git

  • Commit changed from 35e87fcbf81e656137fd13983a9ce3630364e405 to e362c57433b31b26f8d3c1e31e580381ee3b245c

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

e362c57Merge branch 'u/soehms/localization' of git://trac.sagemath.org/sage into localization_28862

comment:7 Changed 17 months ago by soehms

Just fixed merge conflict!

comment:8 Changed 17 months ago by tscrim

Overall this looks good and seems like a feature that should have been in Sage sometime ago. However, I do have some comments:

-Coercion from Localization::
+Coercion from a localization::

I am not sure I like this:

parent.__make_element_class__(FractionFieldEmbedding)

It doesn't feel right to be getting into the internal workings. Homsets are also somewhat special (and we don't have a good mechanism for parents with multiple element classes), so I use one of the following:

parent.element_class(foo)
FractionFieldEmbedding(foo)

I would get rid of this comment:

The implementation of this class is based on an example given in the reference pages on coercion:

`coercion example <http://doc.sagemath.org/html/en/reference/coercion/index.html?highlight=localization#example>`_

and instead just say localizations inherit from IntegralDomain and the consequences. I might also consider moving much of the module-level documentation to the class-level documentation as it is something specific to that implementation and is easier to see from the command line via ?.

I don't think you need to say where the example comes from. So I would remove this:

taken from comment #9 of :trac:`27371`

The items of INPUT: do not end with a period/full-stop. The OUTPUT: should either start with a capital letter or not end with a period/full-stop.

The list is redundant here:

-return sorted(list(set(add_units_result)))
+return sorted(set(add_units_result))

For __init__, a good thing to put in there is a TestSuite(foo).run() call.

Realizes -> Realize (although I would say Compute probably).

is_unit and inverse_of_unit should have at least a one-line description.

In _richcmp_, this test is vacuous:

if self.parent() != other.parent():
    return super(Localization, self)._richcmp_(other, op)

since by the time it gets there, it must be in the same parent. Also

-INPUT (to the constructor):
+INPUT:

Error messages should start with a lowercase letter.

-        Return the i-th generator of self which is
-        the i-th generator of the base ring.
+        Return the ``i``-th generator of ``self``, which is
+        the ``i``-th generator of the base ring.

comment:9 Changed 17 months ago by git

  • Commit changed from e362c57433b31b26f8d3c1e31e580381ee3b245c to 0cbd98533749bf2e71c7ef7aecf8a89868060b62

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

70682e2Merge branch 'u/soehms/localization' of git://trac.sagemath.org/sage into localization_28862
0cbd98528862: corrections according to review

comment:10 Changed 17 months ago by soehms

I could follow all of your suggestions except the one about the definition of the coercion into the field of fraction.

Maybe I didn't get your suggestion, correctly. But, if I try it like this:

+   return FractionFieldEmbedding(S, self, category=parent.homset_category())
-   return parent.__make_element_class__(FractionFieldEmbedding)(S, self, category=parent.homset_category())

I'm getting the following:

sage: R.<x> = QQ[]
sage: L = R.localization(x**2+1)
sage: f = L.fraction_field().coerce_map_from(L)
sage: TestSuite(f).run()
Failure in _test_category:
Traceback (most recent call last):
...
The following tests failed: _test_category

Can we ignore that? An according doctest has been in the code with respect to the coercion from the base ring into its field of fraction.

comment:11 Changed 17 months ago by tscrim

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

Ah, so this pattern has been used in many other places in the Sage code. Thus it is good to do this. I might need to make some appropriate changes to some of my code now that I know about this pattern. Thanks.

comment:12 Changed 17 months ago by tscrim

  • Status changed from positive_review to needs_review

Ah, one more quick thing: the long (>80 chars) lines in the EXAMPLES:: of the module-level doc of localization.py. I thought I had that in my comments. For the code part, it is less important, but you can also have outputs like:

Multivariate Polynomial Ring in u0, u1, u2, q over Integer Ring localized at
 (q, q + 1, u2, u1, u1 - u2, u0, u0 - u2, u0 - u1, u2*q - u1, u2*q - u0,
  u1*q - u2, u1*q - u0, u0*q - u2, u0*q - u1)

In particular, allowing the code parts to break the 80 char/line guideline is okay when it makes the break more natural IMO. Although it is still good to try and follow it.

Once changed, you can set it back to a positive review.

comment:13 Changed 17 months ago by git

  • Commit changed from 0cbd98533749bf2e71c7ef7aecf8a89868060b62 to 44f816ddf5aee524df452434b9e6ffcee72a17bf

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

44f816d28862: docstring formatting

comment:14 Changed 17 months ago by soehms

  • Status changed from needs_review to positive_review

Now, PDF-docs look much nicer! Thanks a lot!

comment:15 Changed 16 months ago by vbraun

  • Branch changed from u/soehms/localization to 44f816ddf5aee524df452434b9e6ffcee72a17bf
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.