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: synthetic geometry #7300

Open
wants to merge 32 commits into
base: master
Choose a base branch
from
Open

feat: synthetic geometry #7300

wants to merge 32 commits into from

Conversation

ah1112
Copy link
Collaborator

@ah1112 ah1112 commented Sep 21, 2023

This is adding synthetic geometry using Avigad's axioms and formalizing Euclid Book I, through the Pythagorean theorem.


Open in Gitpod

@ah1112 ah1112 added help-wanted The author needs attention to resolve issues awaiting-review t-euclidean-geometry Affine and axiomatic geometry labels Sep 21, 2023
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the merge-conflict The PR has a merge conflict with master, and needs manual merging. label Sep 23, 2023
Comment on lines 18 to 43
class incidence_geometry :=
/--Points in the plane-/
(point : Type u)
/--Lines in the plane-/
(line : Type u)
/--Circles in the plane-/
(circle : Type u)

/--A point being on a line-/
(online : point → line → Prop)
/--Two points being on the sameside of a line-/
(sameside : point → point → line → Prop)
/--Three points being in a row-/
(B : point → point → point → Prop)
/--A point being the center of a center-/
(center_circle : point → circle → Prop)
/--A point being on a circle-/
(on_circle : point → circle → Prop)
/--A point being inside a circle-/
(in_circle : point → circle → Prop)
/--That two lines intersect-/
(lines_inter : line → line → Prop)
/--A line and a circle intersect-/
(line_circle_inter : line → circle → Prop)
/--Two circles intersect-/
(circles_inter : circle → circle → Prop)
Copy link
Member

Choose a reason for hiding this comment

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

The mathlib 4 naming guide says these Prop and Type fields should be named as Point, OnCircle, etc, with no underscores and PascalCase.

What you have looks like Lean 3 naming conventions!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, I think I have changed what you requested. What's the next thing you would like me to revise?

Copy link
Contributor

Choose a reason for hiding this comment

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

Another lean4ism is using where instead of := and not using parentheses around the fields. Not sure if it's enforced style.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, I made the parenthesis and := suggestions. What else would you like me to modify?

@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. label Sep 29, 2023
Mathlib/Geometry/Synthetic/Avigad/EuclidBookI.lean Outdated Show resolved Hide resolved
Mathlib/Geometry/Synthetic/Avigad/EuclidBookI.lean Outdated Show resolved Hide resolved
Mathlib/Geometry/Synthetic/Avigad/EuclidBookI.lean Outdated Show resolved Hide resolved
Mathlib/Geometry/Synthetic/Avigad/EuclidBookI.lean Outdated Show resolved Hide resolved
Mathlib/Geometry/Synthetic/Avigad/EuclidBookI.lean Outdated Show resolved Hide resolved
Mathlib/Geometry/Synthetic/Avigad/EuclidBookI.lean Outdated Show resolved Hide resolved
Mathlib/Geometry/Synthetic/Avigad/EuclidBookI.lean Outdated Show resolved Hide resolved
Mathlib/Geometry/Synthetic/Avigad/EuclidBookI.lean Outdated Show resolved Hide resolved
Mathlib/Geometry/Synthetic/Avigad/EuclidBookI.lean Outdated Show resolved Hide resolved
Mathlib/Geometry/Synthetic/Avigad/EuclidBookI.lean Outdated Show resolved Hide resolved
Mathlib/Geometry/Synthetic/Avigad/EuclidBookI.lean Outdated Show resolved Hide resolved
Mathlib/Geometry/Synthetic/Avigad/EuclidBookI.lean Outdated Show resolved Hide resolved
Mathlib/Geometry/Synthetic/Avigad/EuclidBookI.lean Outdated Show resolved Hide resolved
Mathlib/Geometry/Synthetic/Avigad/EuclidBookI.lean Outdated Show resolved Hide resolved
Mathlib/Geometry/Synthetic/Avigad/EuclidBookI.lean Outdated Show resolved Hide resolved
Mathlib/Geometry/Synthetic/Avigad/EuclidBookI.lean Outdated Show resolved Hide resolved
area : Point → Point → Point → ℝ

/--From a set of points getting one additional one-/
more_pts : ∀ (S : Set Point), S.Finite → ∃ a, a ∉ S
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would it make more sense to instead assume that Point is infinite?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

How would you suggest going about this? I am not really sure how to do it.

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
more_pts : ∀ (S : Set Point), S.Finite → ∃ a, a ∉ S
[more_pts : Infinite Point]

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Interesting... How would I use this condition to get the same effect as the original? Previously I could write:
rcases more_pts ∅ Set.finite_empty with ⟨a, -⟩ to get a point, for example.

Copy link
Contributor

Choose a reason for hiding this comment

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

Apparently we're missing a Set.Finite version of Infinite.exists_not_mem_finset?

/--B is strict-/
ne_13_of_B : ∀ {a b c}, B a b c → a ≠ c
/--B is strict-/
ne_23_of_B : ∀ {a b c}, B a b c → b ≠ c
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this condition is redundant

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I got rid of ne_23_of_B since it can be deduced from ne_12_of_B. I don't think ne_13_of_B is redundant though?

/--Degenerate areas are zero-/
degenerate_area : ∀ a b, area a a b = 0
/--Area is completely symmetric-/
area_invariant : ∀ a b c, area a b c = area c a b ∧ area a b c = area a c b
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think usually we would break this up into two separate conditions

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, I have done so now!


/-- `IncidenceGeometry` represents geometry in the Euclidean sense, with primitives for points
lines and circles-/
class IncidenceGeometry where
Copy link
Member

Choose a reason for hiding this comment

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

Everything after this where should be indented by two spaces

Copy link
Collaborator Author

@ah1112 ah1112 Oct 13, 2023

Choose a reason for hiding this comment

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

Done

Copy link
Member

Choose a reason for hiding this comment

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

I see 4 spaces, not 2.

open IncidenceGeometry
-------------------------------------------------- Definitions -----------------------------------
/--Points being on different sides of a line-/
def diffside a b L := ¬OnLine a L ∧ ¬OnLine b L ∧ ¬SameSide a b L
Copy link
Member

@eric-wieser eric-wieser Oct 13, 2023

Choose a reason for hiding this comment

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

Thee should be written as

Suggested change
def diffside a b L := ¬OnLine a L ∧ ¬OnLine b L ∧ ¬SameSide a b L
def DiffSide (a b : Point) (L : Line) : Prop :=
¬OnLine a L ∧ ¬OnLine b L ∧ ¬SameSide a b L

note:

  • Explicit type annotations
  • Use of CamelCase for Prop definitions

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Wait, should the explicit type annotations also be true for the axioms? Or for theorems?

Copy link
Member

Choose a reason for hiding this comment

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

I've only looked in detail at this file so far, so for now just these defs

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

/--Getting a circle with center and point on it-/
circle_of_ne : ∀ {a b}, a ≠ b → ∃ α, CenterCircle a α ∧ OnCircle b α
/--If lines intersect then this gives you the point of intersection-/
pt_of_linesinter : ∀ {L M}, LinesInter L M → ∃ a, OnLine a L ∧ OnLine a M
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
pt_of_linesinter : ∀ {L M}, LinesInter L M → ∃ a, OnLine a L ∧ OnLine a M
pt_of_linesInter : ∀ {L M}, LinesInter L M → ∃ a, OnLine a L ∧ OnLine a M

If a theorem is about FooBar x, it should have fooBar in its name not foobar. I think the naming guide explains this a little better than I have.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I see... Should the theorem then be Point_of_LinesInter instead?

Copy link
Member

Choose a reason for hiding this comment

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

No, they should be point_of_lineInter; the first letter is kept lowercase. It's somewhat arbitrary, but it's what the style guidelines say, and following them makes things consistent.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done I think?

(angle b a c = angle e d f ↔ length b c = length e f)

variable [i : IncidenceGeometry]
open IncidenceGeometry
Copy link
Member

Choose a reason for hiding this comment

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

I think you probably want

Suggested change
open IncidenceGeometry
namespace IncidenceGeometry

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok! Just out of curiosity, why do we want to do this? I changed it in the Axioms file and in EuclidBookI, but was not able to do it for Tactics since the elab expressions started to break?

Copy link
Member

Choose a reason for hiding this comment

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

Again, for now this advice is only for this axioms file. It means that the axioms end up in the same namespace as the derived definitions.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

Comment on lines 18 to 24
class IncidenceGeometry where
/--Points in the plane-/
Point : Type u
/--Lines in the plane-/
Line : Type u
/--Circles in the plane-/
Circle : Type u
Copy link
Member

@eric-wieser eric-wieser Oct 22, 2023

Choose a reason for hiding this comment

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

I suspect this typeclass will be more usable as

Suggested change
class IncidenceGeometry where
/--Points in the plane-/
Point : Type u
/--Lines in the plane-/
Line : Type u
/--Circles in the plane-/
Circle : Type u
class IncidenceGeometry (Point : Type u) where
/--Lines in the plane-/
Line : Type u
/--Circles in the plane-/
Circle : Type u

because then we can add an instance that says "euclidean space satisfies the axioms of "incidence geometry".

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

When I try to change this, variable [i : IncidenceGeometry] breaks, along with anything that requires points. How do I get around this?

Copy link
Member

Choose a reason for hiding this comment

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

You change it to {Point : Type*} [IncidenceGeometry Point]

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok. It seems if I do that, then I would have to write i.Line or i.rightangle every time? Is there some way to avoid this?

@eric-wieser eric-wieser added awaiting-author A reviewer has asked the author a question or requested changes and removed awaiting-review labels Oct 22, 2023
Comment on lines 216 to 217
/--Points being on different sides of a line-/
def DiffSide (a b : Point) (L : Line) := ¬OnLine a L ∧ ¬OnLine b L ∧ ¬SameSide a b L
Copy link
Member

@eric-wieser eric-wieser Oct 22, 2023

Choose a reason for hiding this comment

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

Please add explicit types here. I think for all of the below, the explicit type is

Suggested change
/--Points being on different sides of a line-/
def DiffSide (a b : Point) (L : Line) := ¬OnLine a L ∧ ¬OnLine b L ∧ ¬SameSide a b L
/--Points being on different sides of a line-/
def DiffSide (a b : Point) (L : Line) : Prop := ¬OnLine a L ∧ ¬OnLine b L ∧ ¬SameSide a b L

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

My bad! Will do

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@ah1112 ah1112 added awaiting-review and removed awaiting-author A reviewer has asked the author a question or requested changes labels Nov 27, 2023
/--The area made by three points-/
area : Point → Point → Point → ℝ
/--From a set of points getting one additional one-/
more_pts : ∀ (S : Set Point), S.Finite → ∃ a, a ∉ S
Copy link
Member

Choose a reason for hiding this comment

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

The Mathlib way to say this is Infinite Point.

universe u

/-- `IncidenceGeometry` represents geometry in the Euclidean sense, with primitives for points
lines and circles-/
Copy link
Member

Choose a reason for hiding this comment

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

Do you use some specific book as the source for the list of axioms? If yes, then this should be mentioned both in the module docstring and in the typeclass docstring.

lines and circles-/
class IncidenceGeometry where
/--Points in the plane-/
Point : Type u
Copy link
Member

Choose a reason for hiding this comment

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

If Point is a data field of the typeclass, not an argument, then we can't have 2 instances in the same universe without having lots of issues.

/--Angles are nonnegative-/
angle_nonneg : ∀ a b c, 0 ≤ angle a b c
/--Lengths are nonnegative-/
length_nonneg : ∀ a b, 0 ≤ length a b
Copy link
Member

Choose a reason for hiding this comment

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

I see that you don't require the triangle inequality here. What implies it? Can we extend MetricSpace X instead of introducing domain-specific length? Note that the goal of Mathlib is to have different parts of the library play well with each other.

@urkud urkud added awaiting-author A reviewer has asked the author a question or requested changes and removed awaiting-review labels Jan 23, 2024
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 help-wanted The author needs attention to resolve issues t-euclidean-geometry Affine and axiomatic geometry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants