sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.37k stars 466 forks source link

Enhance global_height functionality for other fields #28071

Closed b11e3d19-0e1d-4b36-b44b-4e039832fbf9 closed 5 years ago

b11e3d19-0e1d-4b36-b44b-4e039832fbf9 commented 5 years ago

Currently we cannot calculate the global height of an element that is not already an explicit element of QQ, QQbar, or NumberField. For example,

sage: P.<x,y> = ProjectiveSpace(AlgebraicRealField(), 1)
sage: Q = P(10)
sage: Q.global_height()

currently fails, when we should be able to calculate the global heights for a point in any field that can be embedded into QQbar. We use coercion to QQbar in the global_height() function to solve this problem. There are also numerous instances in the projective_ds file where a check is made on the base field for the sole purpose of safely calling the global_height() function. We have removed these checks in favor of a single check in the global_height() function.

CC: @bhutz

Component: dynamics

Keywords: SI2019

Author: Talia Blum, Trevor Hyde, Joey Lupo, Matt Torrence

Branch/Commit: 667039f

Reviewer: Shuofeng Xu, Max Weinreich, Brandon Gontmacher, Heidi Benham, Ben Hutz

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

b11e3d19-0e1d-4b36-b44b-4e039832fbf9 commented 5 years ago

Changed author from Trevor Hyde, Joey Lupo, Matt Torrence to Talia Blum, Trevor Hyde, Joey Lupo, Matt Torrence

b11e3d19-0e1d-4b36-b44b-4e039832fbf9 commented 5 years ago

Description changed:

--- 
+++ 
@@ -1,9 +1,9 @@
-Currently we cannot calculate the global height of an element that is not already an explicit element of QQ, QQbar, or NumberField. For example,
+Currently we cannot calculate the global height of an element that is not already an explicit element of `QQ`, `QQbar`, or `NumberField`. For example,

-P.<x,y> = ProjectiveSpace(AlgebraicRealField(), 1) -Q = P(10) -Q.global_height() +sage: P.<x,y> = ProjectiveSpace(AlgebraicRealField(), 1) +sage: Q = P(10) +sage: Q.global_height()


-currently fails, when we should be able to calculate the global heights for a point in any field that can be embedded into QQbar. We use coercion to QQbar in the global_heights function to solve this problem. There are also numerous instances in the projective_ds file where a check is made on the base field for the sole purpose of safely calling the global_height() function. We have removed these checks in favor of a single check in the global_height function.
+currently fails, when we should be able to calculate the global heights for a point in any field that can be embedded into `QQbar`. We use coercion to `QQbar` in the `global_height()` function to solve this problem. There are also numerous instances in the `projective_ds` file where a check is made on the base field for the sole purpose of safely calling the `global_height()` function. We have removed these checks in favor of a single check in the `global_height()` function.
b11e3d19-0e1d-4b36-b44b-4e039832fbf9 commented 5 years ago

Commit: 01747de

b11e3d19-0e1d-4b36-b44b-4e039832fbf9 commented 5 years ago

Branch: u/gh-Torrencem/28071_global_height

b11e3d19-0e1d-4b36-b44b-4e039832fbf9 commented 5 years ago

New commits:

01747de28071: fix global height checks
39bf98b2-f392-42d4-a7cb-688d50feb567 commented 5 years ago

Reviewer: Shuofeng Xu, Max Weinreich, Brandon Gontmacher, Heidi Benham

bhutz commented 5 years ago
comment:5

Two questions here:

1) Can the new ``except'' trap a specific error?

2) It would be better if the example had elements actually in the field you are defining instead of just over QQ.

vbraun commented 5 years ago

Changed branch from u/gh-Torrencem/28071_global_height to 01747de

vbraun commented 5 years ago

Changed commit from 01747de to none

4edfbb98-6b40-4472-8655-ea92584e13fe commented 5 years ago

Changed branch from 01747de to u/gh-Torrencem/28071_global_height

4edfbb98-6b40-4472-8655-ea92584e13fe commented 5 years ago
comment:8

The latest commit should fix these issues Ben


New commits:

01747de28071: fix global height checks
667039f28071: Change example, more specific except
4edfbb98-6b40-4472-8655-ea92584e13fe commented 5 years ago

Commit: 667039f

bhutz commented 5 years ago

Changed reviewer from Shuofeng Xu, Max Weinreich, Brandon Gontmacher, Heidi Benham to Shuofeng Xu, Max Weinreich, Brandon Gontmacher, Heidi Benham, Ben Hutz

vbraun commented 5 years ago

Changed branch from u/gh-Torrencem/28071_global_height to 667039f