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

feat(algebraic_geometry): descent theorem #10830

Closed
wants to merge 23 commits into from
Closed

Conversation

jjaassoonn
Copy link
Collaborator


Open in Gitpod

@alexjbest
Copy link
Member

Maybe you already saw it, but I posted some old code I had on this sort of material to Zulip today at https://leanprover.zulipchat.com/#narrow/stream/116395-maths/topic/thoughts.20on.20elliptic.20curves/near/265022015, its not the most general definition of a height function, and so the proof is easier than the one in Silverman say. But if it is at all useful to you feel free to reuse parts of it.

@jjaassoonn
Copy link
Collaborator Author

This proof is now sorry free, but I wonder if it is worth writing it in mathlib standard

@jjaassoonn jjaassoonn marked this pull request as ready for review December 17, 2021 23:46
@alexjbest
Copy link
Member

This proof is now sorry free, but I wonder if it is worth writing it in mathlib standard

Nice! Personally I think it is worth adding to mathlib, its a really important result. I'm not sure what uses it has outside of proving the Mordell-Weil theorem, but that alone should be enough to justify adding it to mathlib IMO.

@jjaassoonn jjaassoonn changed the title trying to prove descent theorem feat(algebraic_geometry): descent theorem Dec 18, 2021
@jjaassoonn
Copy link
Collaborator Author

Can you add this for submonoid, add_subgroup, add_submonoid, subsemiring, and semiring too? Maybe in a new PR with the whole set.

Is it like #10908?

@github-actions github-actions bot added the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Dec 20, 2021
@github-actions github-actions bot removed the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Dec 20, 2021
@semorrison semorrison added the awaiting-review The author would like community review of the PR label Feb 27, 2022

universe u

variable (A : Ab.{u})
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason why you use A : Ab instead of [comm_group _]?

Copy link
Member

@jcommelin jcommelin left a comment

Choose a reason for hiding this comment

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

This will need better documentation, and better names (and namespacing) before it is ready for further review.


variable (A : Ab.{u})

/--definition of height function-/
Copy link
Member

Choose a reason for hiding this comment

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

Please write a more detailed docstring, with capital letters and interpunction. Maybe add a reference to another source.

include fin_quot

/--the $Q_i$ in Silverman's book-/
def represents : finset A :=
Copy link
Member

Choose a reason for hiding this comment

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

It seems that this is in the root namespace. Is that intentional?

end

variables {A}
lemma represents_represent_A_quot_mA :
Copy link
Member

Choose a reason for hiding this comment

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

Names should try to avoid referring to variables such as A.

@jcommelin jcommelin added needs-documentation This PR is missing required documentation awaiting-author A reviewer has asked the author a question or requested changes and removed awaiting-review The author would like community review of the PR labels Mar 4, 2022
@leanprover-community-bot-assistant leanprover-community-bot-assistant added the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Mar 15, 2022
@jjaassoonn jjaassoonn closed this May 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-author A reviewer has asked the author a question or requested changes merge-conflict Please `git merge origin/master` then a bot will remove this label. needs-documentation This PR is missing required documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants