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

Card lemmas #509

Merged
merged 7 commits into from Jun 19, 2020
Merged

Card lemmas #509

merged 7 commits into from Jun 19, 2020

Conversation

chdoc
Copy link
Member

@chdoc chdoc commented May 14, 2020

Motivation for this change

Additional lemmas about cardinalities of predicates and sets. Mainly picking n distinct elements for an a predicate with n <= #|A|.

Things done/to do
  • added corresponding entries in CHANGELOG_UNRELEASED.md
  • added corresponding documentation in the headers
Automatic note to reviewers

Read this Checklist and make sure there is a milestone.

@@ -624,13 +624,17 @@ Proof.
by apply: (iffP idP) => [/eqP/mem_card1[x inA]|[x /eq_card1/eqP//]]; exists x.
Qed.

Lemma fintype_le1P : reflect (forall x, all_equal_to x) (#|T| <= 1).
Lemma card_le1_eqP A :
Copy link
Member

Choose a reason for hiding this comment

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

In thought I did that one when I added fintype_le1P tomorrow. 😆 I will double check tomorrow, if I forgot to push something.

@@ -824,6 +824,16 @@ rewrite eq_sym eqn_leq card_gt0 => /andP[/set0Pn[x Ax] leA1].
by exists x; apply/eqP; rewrite eq_sym eqEcard sub1set Ax cards1 leA1.
Qed.

Lemma cards2P A : reflect (exists x y : T, x != y /\ A = [set x;y]) (#|A| == 2).
Copy link
Member

Choose a reason for hiding this comment

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

Nice one to have indeed, I think there is a lot of refactoring to do with all the card_* lemma you added, through enum (which is uniq which size is the cardinal of the considered set). Indeed, I think card_eqP would be almost a one liner and card_geqP, and cards2P is a simple twice case analysis (the main step being something like case: (enum A) (enum_uniq A) (size_enum A) => [|x [|y []]//; exists x; exists y).
I also think it would be nice to have le/lt variants and pred variants.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure I follow. There is no card_eqP (yet) and indeed I'm not sure that would be useful, since enum A has already good lemma support. And card_geqP is about encapsulating the picking of a subsequence of enum A, if one only has a lower bound.
Admittedly, using the _gt?P lemmas to prove cards2P may be overkill.

Copy link
Member

Choose a reason for hiding this comment

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

Here is an elaboration of what I had in mind:

Lemma set_enum A : [set x | x \in enum A] = A.
Proof. by apply/setP => x; rewrite inE mem_enum. Qed.

Variant cards_eq_spec A : seq T -> {set T} -> nat -> Type :=
| CardEq (s : seq T) & uniq s : cards_eq_spec A s [set x | x \in s] (size s).

Lemma cards_eqP A : cards_eq_spec A (enum A) A #|A|.
Proof.
by move: (enum A) (cardE A) (set_enum A) (enum_uniq A) => s -> <-; constructor.
Qed.

Lemma cards1P A : reflect (exists x, A = [set x]) (#|A| == 1).
Proof.
apply: (iffP idP) => [|[x ->]]; last by rewrite cards1.
by have [[|x []]// _] := cards_eqP; exists x; apply/setP => y; rewrite !inE.
Qed.

Lemma cards2P A : reflect (exists x y : T, x != y /\ A = [set x; y]) (#|A| == 2).
Proof.
apply: (iffP idP) => [|[x] [y] [xy ->]]; last by rewrite cards2 xy.
have [[|x [|y []]]//=] := cards_eqP; rewrite !inE andbT => neq_xy.
by exists x, y; split=> //; apply/setP => z; rewrite !inE.
Qed.

and something like this for fintype...

Copy link
Member

Choose a reason for hiding this comment

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

And card_geqP is about encapsulating the picking of a subsequence of enum A, if one only has a lower bound.

Indeed, I missed that...

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, for {set _} what you propose is indeed more general and more direct than what I had proposed. However, I'm not sure it transfers all that well to predicates: due to the lack of extensionality, one cannot pull the [set x | x \in enum A] trick. One could instead generate s =i A as an assumption, but I'm not sure this view would be all that useful.

The same goes for lower bounds, albeit for a different reason: there one doesn't even want to replace the original predicate/set.

@affeldt-aist affeldt-aist added this to the 1.12.0 milestone May 15, 2020
@chdoc
Copy link
Member Author

chdoc commented May 26, 2020

I tried to incorporate the feedback from @CohenCyril , and I found two more lemmas fcard_gt0P and fcard_gt1P. The proof of the latter naturally makes use of card_gt1P. These are adapted/simplified from the fourcolor development, where one can find a specific instance for each of them (fcard0P and fcard1P).

Copy link
Member

@CohenCyril CohenCyril left a comment

Choose a reason for hiding this comment

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

LGTM, except for minor proof style or spacing issues.
I made many code suggestions to point them out with a suggest fix, which I did not test

mathcomp/ssreflect/seq.v Show resolved Hide resolved
mathcomp/ssreflect/fintype.v Outdated Show resolved Hide resolved
mathcomp/ssreflect/fintype.v Outdated Show resolved Hide resolved
mathcomp/ssreflect/fintype.v Outdated Show resolved Hide resolved
mathcomp/ssreflect/fintype.v Outdated Show resolved Hide resolved
mathcomp/ssreflect/fingraph.v Outdated Show resolved Hide resolved
mathcomp/ssreflect/fingraph.v Show resolved Hide resolved
mathcomp/ssreflect/fintype.v Outdated Show resolved Hide resolved
mathcomp/ssreflect/fintype.v Outdated Show resolved Hide resolved
mathcomp/ssreflect/fintype.v Outdated Show resolved Hide resolved
@coqbot coqbot added the needs: rebase PR which is not rebased: check the target is appropriate (generally master) and rebase on top of it. label Jun 7, 2020
@coqbot coqbot removed the needs: rebase PR which is not rebased: check the target is appropriate (generally master) and rebase on top of it. label Jun 7, 2020
@chdoc chdoc requested a review from CohenCyril June 7, 2020 12:35
@coqbot coqbot added the needs: rebase PR which is not rebased: check the target is appropriate (generally master) and rebase on top of it. label Jun 17, 2020
@coqbot coqbot removed the needs: rebase PR which is not rebased: check the target is appropriate (generally master) and rebase on top of it. label Jun 17, 2020
CohenCyril
CohenCyril previously approved these changes Jun 17, 2020
@CohenCyril CohenCyril added kind: enhancement Issue or PR about addition of features. kind: refactoring Issue or PR about a refactoring. (reorganizing the code, reusing theorems, simplifications...) labels Jun 17, 2020
@chdoc
Copy link
Member Author

chdoc commented Jun 17, 2020

I wasn't sure about this, because the line already had 82 characters, now it gained another 6. And yes, I want a linter, too. 😄

@CohenCyril
Copy link
Member

I wasn't sure about this, because the line already had 82 characters, now it gained another 6. And yes, I want a linter, too.

Oh ok, I should really checkout locally.
In the core mathcomp repo there is a strict rule about the 80 characters per line limit. So this line should be split anyway.

@CohenCyril CohenCyril dismissed their stale review June 17, 2020 12:26

still some typesetting to do

@chdoc
Copy link
Member Author

chdoc commented Jun 17, 2020

Well, running a grep -n '.\{81\}' ssreflect/*.v made me believe that the rule is not that strict.

@CohenCyril
Copy link
Member

CohenCyril commented Jun 17, 2020

Well, running a grep -n '.\{81\}' ssreflect/*.v made me believe that the rule is not that strict.

It used to be, this is outrageous 😉
I will revive #163 ASAP.

@coqbot coqbot added the needs: rebase PR which is not rebased: check the target is appropriate (generally master) and rebase on top of it. label Jun 18, 2020
@coqbot coqbot removed the needs: rebase PR which is not rebased: check the target is appropriate (generally master) and rebase on top of it. label Jun 18, 2020
Copy link
Member

@CohenCyril CohenCyril left a comment

Choose a reason for hiding this comment

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

Yeay, LGTM now!

@CohenCyril CohenCyril merged commit f25ef67 into math-comp:master Jun 19, 2020
@chdoc chdoc mentioned this pull request Jun 26, 2020
2 tasks
@chdoc chdoc deleted the card-lemmas branch June 30, 2020 11:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: enhancement Issue or PR about addition of features. kind: refactoring Issue or PR about a refactoring. (reorganizing the code, reusing theorems, simplifications...)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants