Global function fields: places

[kwankyu]

This is part of the meta-ticket #22982.

The goal of the present ticket is to add code for computing places of global function fields.

Component: algebra

Author: Kwankyu Lee

Branch/Commit: 8049919

Reviewer: Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/26616

[kwankyu]

Branch: u/klee/26616

[sagetrac-git]

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

2531d4a

Add place

[sagetrac-git]

Commit: 2531d4a

[sagetrac-git]

Changed commit from 2531d4a to 2e6ebee

[sagetrac-git]

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

a5b1f66	pyflakes fixes
2e6ebee	Fixes for python3 compatibility

[sagetrac-git]

Changed commit from 2e6ebee to b0773fb

[sagetrac-git]

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

b0773fb

Remove unused _cache

[sagetrac-git]

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

1d8d0a6

Remove unnecessry @cahced_method

[sagetrac-git]

Changed commit from b0773fb to 1d8d0a6

[kwankyu]

Author: Kwankyu Lee

[sagetrac-git]

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

ecc7405

Remove unused import

[sagetrac-git]

Changed commit from 1d8d0a6 to ecc7405

[sagetrac-git]

Changed commit from ecc7405 to b117c23

[sagetrac-git]

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

b117c23

Merge branch 'develop'

[sagetrac-git]

Changed commit from b117c23 to b23cc8e

[sagetrac-git]

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

b23cc8e

Fix doctests that randomly fail

[sagetrac-git]

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

d83ddd0

Merge branch 'develop'

[sagetrac-git]

Changed commit from b23cc8e to d83ddd0

[sagetrac-git]

Changed commit from d83ddd0 to 69f866f

[sagetrac-git]

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

69f866f

Merge branch 'develop'

[tscrim]

comment:13

Some comments:

• Perhaps it is my lack of understanding of the field, but is place common terminology for valuation()? It might be good to keep some indication about it being a prime element.

• What is the reason for FunctionFieldElement_global.__cached_methods?

• You should be able to replace [place for place in self._places_finite(degree)] with list(self._places_finite(degree)). Similarly for the infinite.

• I think there is a grammar issue here: Return the unique place at infinite. Should it be Return the unique place at infinity.?

• Instead of a @cached_method for _gens_over_base, I could consider making it a @lazy_attribute. (So you then use it as self._gens_over_base instead of self._gens_over_base().)

• I am not sure about this change:

  -        norm = reduce(operator.mul, hnf.diagonal())
  +        norm = 1
  +        for e in hnf.diagonal():
  +            norm *= e

  as it might be slower. However, this is a minor thing (the new code is definitely more readable).

• I do not think you need the lazy_import('sage.matrix.constructor', 'matrix') in sage/rings/function_field/valuation_ring.py.

Otherwise it does what it says it does, so LGTM.

[kwankyu]

comment:14

Replying to @tscrim:

Some comments:

• Perhaps it is my lack of understanding of the field, but is place common terminology for valuation()? It might be good to keep some indication about it being a prime element.

"place" is established terminology in the function field theory. Places correspond, one-to-one, to (discrete) valuation rings of the function field, which defines valuation of the elements of the function field. In a later stage, I will introduce "divisors", which are central objects in the function field theory. A divisor is defined to be a formal sum of places. A place is not said to be prime, though "finite" places are in one-to-one correspondence with prime ideals of the "finite" maximal order.

• What is the reason for FunctionFieldElement_global.__cached_methods?

I forgot :-) I removed it.

• You should be able to replace [place for place in self._places_finite(degree)] with list(self._places_finite(degree)). Similarly for the infinite.

Done. Thanks.

• I think there is a grammar issue here: Return the unique place at infinite. Should it be Return the unique place at infinity.?

Right. Fixed.

• Instead of a @cached_method for _gens_over_base, I could consider making it a @lazy_attribute. (So you then use it as self._gens_over_base instead of self._gens_over_base().)

Ok. Done. Thanks for a good tip.

• I am not sure about this change:

  -        norm = reduce(operator.mul, hnf.diagonal())
  +        norm = 1
  +        for e in hnf.diagonal():
  +            norm *= e

  as it might be slower. However, this is a minor thing (the new code is definitely more readable).

I guess Guido did not remove reduce in python3 for no reason. I did a simple experiment (with ints) in python3 and both take the same time. I would prefer to remove reduce.

• I do not think you need the lazy_import('sage.matrix.constructor', 'matrix') in sage/rings/function_field/valuation_ring.py.

Removed.

Thank you for giving attention!!

[sagetrac-git]

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

1409127	Remove unused __cached_methods
bcc330f	Merge branch 'develop' into function-fields-3-trac26616
8e86177	Fixes for reviewer comments

[sagetrac-git]

Changed commit from 69f866f to 8e86177

[sagetrac-git]

Changed commit from 8e86177 to 97d3bc3

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

97d3bc3

Add places to global function fields

[kwankyu]

comment:17

Squashed and rebased to sage 8.6.

[tscrim]

Reviewer: Travis Scrimshaw

[tscrim]

comment:18

Thank you for the explanation about the definition of a place. I learned something new. <sup>_</sup>

Do you think you could add that or some variant to place.py, either at the module-level or in some key class-level doc? Maybe also with a standard reference for the subject? Once that is done, positive review.

I forgot that reduce was moved to functools. Thanks for the reminder.

[sagetrac-git]

Changed commit from 97d3bc3 to 8049919

[sagetrac-git]

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

8049919

Add some explanation of basic concepts and a reference

[tscrim]

comment:20

Thank you.

[vbraun]

Changed branch from u/klee/26616 to 8049919