Opened 4 years ago

Closed 4 years ago

#25821 closed enhancement (fixed)

implement height functions for product points

Reported by: Raghukul Raman Owned by:
Priority: minor Milestone: sage-8.3
Component: algebraic geometry Keywords: gsoc2018
Cc: Ben Hutz, Raghukul Raman Merged in:
Authors: Raghukul Raman Reviewers: Ben Hutz
Report Upstream: N/A Work issues:
Branch: af73f95 (Commits, GitHub, GitLab) Commit: af73f95756065d5f07335c71e1b293cefba9c08f
Dependencies: Stopgaps:

Status badges

Description

Currently local_height() and global_height() function are not present in the product projective points.

Change History (7)

comment:1 Changed 4 years ago by Raghukul Raman

Branch: u/raghukul01/implement_height_functions_for_product_points

comment:2 Changed 4 years ago by Raghukul Raman

Authors: Raghukul Raman
Commit: 44a87820170d15b234392d662c531022e955674d
Status: newneeds_review

New commits:

44a878225821: Added global_height and local height for product points

comment:3 Changed 4 years ago by Ben Hutz

Concept is fine, but the code can be made more readable

return max([self[i].global_height(prec=prec) \
                     for i in range(self.codomain().ambient_space().num_components())])

to

return max([t.global_height(prec=prec) for t in self])

I think more explanation is warranted in the docs as well. i.e., that you are taking the max of the heights of the component points.

and similarly for local height.

comment:4 Changed 4 years ago by git

Commit: 44a87820170d15b234392d662c531022e955674daf73f95756065d5f07335c71e1b293cefba9c08f

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

af73f9525821: Updated docstring

comment:5 in reply to:  3 Changed 4 years ago by Raghukul Raman

I think return max([t.global_height(prec=prec) for t in self] won't work since __iter__() function iterates over all coordinates, but here we need to iterate over all components. If we take max over all coordinates then we need to add a function to convert the space defined over QQbar to NumberField, as done in projective_point.

I have made it more readable though.

Replying to bhutz:

Concept is fine, but the code can be made more readable

return max([self[i].global_height(prec=prec) \
                     for i in range(self.codomain().ambient_space().num_components())])

to

return max([t.global_height(prec=prec) for t in self])

I think more explanation is warranted in the docs as well. i.e., that you are taking the max of the heights of the component points.

and similarly for local height.

comment:6 Changed 4 years ago by Ben Hutz

Reviewers: Ben Hutz
Status: needs_reviewpositive_review

comment:7 Changed 4 years ago by Volker Braun

Branch: u/raghukul01/implement_height_functions_for_product_pointsaf73f95756065d5f07335c71e1b293cefba9c08f
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.