Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Height function for projective subvarieties #36094

Merged
merged 4 commits into from
Aug 27, 2023

Conversation

MatheMagicianPi
Copy link
Contributor

@MatheMagicianPi MatheMagicianPi commented Aug 16, 2023

Introduces three new methods related to height of projective subvarieties.

Direct transfer from #34559. Methods implemented: local_height, local_height_arch, global_height.

Fixes #34559

📝 Checklist

  • The title is concise, informative, and self-explanatory.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • Someone else has created tests covering the changes.
  • Someone else has updated the documentation accordingly.

⌛ Dependencies

@kwankyu
Copy link
Collaborator

kwankyu commented Aug 17, 2023

Is @guojing0 the author of the code, but you @MatheMagicianPi created this PR in behalf of him?

If so, then did you notice him personally about this?

I am asking these because I wonder how credits problem should be handled.

@EnderWannabe
Copy link
Contributor

Hi @kwankyu, you are right that @guojing0 is the author of the code. These functions began as part of his GSoC 2022 project on height functions which I helped supervise. I prompted Dang (@MatheMagicianPi) to create this pull request on his behalf to try and close this issue, as Jing is currently working on other issues. I'm happy to defer to your judgement on how code should be credited.

@kwankyu
Copy link
Collaborator

kwankyu commented Aug 19, 2023

Some people use "Assignees" to give credits to all those involved in a PR resolving an issue. It is unfortunate that it is not "Contributors" or "Authors", but it seem the best we can do in the GitHub system. So I assigned both of them to "Assignees".

By the way, I think it is all right that you review this PR unless you prefer others.

default RealField precision).
OUTPUT:
- a real number.
EXAMPLES::
Copy link
Collaborator

@kwankyu kwankyu Aug 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are no spaces separating parts of the docstring here and below.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The standard in

https://doc.sagemath.org/html/en/developer/coding_basics.html#template

(also many examples in the file being modified) suggests blank lines just below INPUT: and OUTPUT:.

@kwankyu
Copy link
Collaborator

kwankyu commented Aug 22, 2023

Thanks.

On more thing: as RealField is a piece of code, we write

``RealField``

Otherwise, it looks good to me.

@kwankyu kwankyu mentioned this pull request Aug 23, 2023
5 tasks
@github-actions
Copy link

Documentation preview for this PR (built with commit bf3b9a1; changes) is ready! 🎉

Copy link
Collaborator

@kwankyu kwankyu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. LGTM.

@vbraun vbraun merged commit 727a619 into sagemath:develop Aug 27, 2023
11 of 12 checks passed
@mkoeppe mkoeppe added this to the sage-10.2 milestone Aug 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Height function for projective subvarieties
6 participants