Opened 2 years ago

Closed 2 years ago

#28071 closed enhancement (fixed)

Enhance global_height functionality for other fields

Reported by: gh-Loops7 Owned by:
Priority: minor Milestone: sage-8.9
Component: dynamics Keywords: SI2019
Cc: bhutz Merged in:
Authors: Talia Blum, Trevor Hyde, Joey Lupo, Matt Torrence Reviewers: Shuofeng Xu, Max Weinreich, Brandon Gontmacher, Heidi Benham, Ben Hutz
Report Upstream: N/A Work issues:
Branch: 667039f (Commits, GitHub, GitLab) Commit: 667039ff3e460bd28222dd5a107e4ba086dfe669
Dependencies: Stopgaps:

Status badges

Description (last modified by gh-Loops7)

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.

Change History (10)

comment:1 Changed 2 years ago by gh-Loops7

  • Authors changed from Trevor Hyde, Joey Lupo, Matt Torrence to Talia Blum, Trevor Hyde, Joey Lupo, Matt Torrence
  • Description modified (diff)

comment:2 Changed 2 years ago by gh-Loops7

  • Branch set to u/gh-Torrencem/28071_global_height
  • Commit set to 01747de179bdece08e18e308ea9152ef2c496e96
  • Summary changed from global_height does not accept an AlgebraicRealField, UniversalCyclotomic to Enhance global_height functionality for other fields

New commits:

​01747de28071: fix global height checks

comment:3 Changed 2 years ago by gh-Loops7

  • Status changed from new to needs_review

comment:4 Changed 2 years ago by gh-ksldr

  • Reviewers set to Shuofeng Xu, Max Weinreich, Brandon Gontmacher, Heidi Benham
  • Status changed from needs_review to positive_review

comment:5 Changed 2 years ago by bhutz

  • Status changed from positive_review to needs_work

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.

comment:6 Changed 2 years ago by vbraun

  • Branch changed from u/gh-Torrencem/28071_global_height to 01747de179bdece08e18e308ea9152ef2c496e96
  • Resolution set to fixed
  • Status changed from needs_work to closed

comment:7 Changed 2 years ago by vbraun

  • Commit 01747de179bdece08e18e308ea9152ef2c496e96 deleted
  • Resolution fixed deleted
  • Status changed from closed to new

comment:8 Changed 2 years ago by gh-Torrencem

  • Branch changed from 01747de179bdece08e18e308ea9152ef2c496e96 to u/gh-Torrencem/28071_global_height
  • Commit set to 667039ff3e460bd28222dd5a107e4ba086dfe669
  • Status changed from new to needs_review

The latest commit should fix these issues Ben

New commits:

​01747de28071: fix global height checks
​667039f28071: Change example, more specific except

comment:9 Changed 2 years ago by bhutz

  • Reviewers changed from Shuofeng Xu, Max Weinreich, Brandon Gontmacher, Heidi Benham to Shuofeng Xu, Max Weinreich, Brandon Gontmacher, Heidi Benham, Ben Hutz
  • Status changed from needs_review to positive_review

comment:10 Changed 2 years ago by vbraun

  • Branch changed from u/gh-Torrencem/28071_global_height to 667039ff3e460bd28222dd5a107e4ba086dfe669
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.