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

[Merged by Bors] - chore: fix some Set defeq abuse, golf #6114

Closed
wants to merge 4 commits into from

Conversation

urkud
Copy link
Member

@urkud urkud commented Jul 24, 2023

  • Use {x | p x} instead of fun x ↦ p x to define a set here and there.
  • Golf some proofs.
  • Replace Con.ker_apply_eq_preimage with Con.ker_apply. The old version used to abuse definitional equality between Set M and M → Prop.
  • Fix Submonoid.mk* lemmas to use ⟨_, _⟩, not ⟨⟨_, _⟩, _⟩.

I found these while trying to make Set a one-field structure. While I'm not sure that I'll complete that other refactor, these changes seem good to me in any case.

Open in Gitpod

@urkud urkud added the awaiting-review The author would like community review of the PR label Jul 24, 2023
Mathlib/Data/Set/Image.lean Outdated Show resolved Hide resolved
@@ -92,7 +92,7 @@ theorem evalFrom_of_append (start : σ) (x y : List α) :
#align DFA.eval_from_of_append DFA.evalFrom_of_append

/-- `M.accepts` is the language of `x` such that `M.eval x` is an accept state. -/
def accepts : Language α := fun x => M.eval x ∈ M.accept
def accepts : Language α := {x | M.eval x ∈ M.accept}
Copy link
Member

Choose a reason for hiding this comment

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

This is still a defeq abuse because Language and Set are not interchangeable.
leanprover-community/mathlib#18451 had a path to resolve this, but was more trouble that I cared to deal with.

Having said that, this change is certainly not harmful, so fine to leave it

Copy link
Member Author

Choose a reason for hiding this comment

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

They are not interchangeable but Language is defined as Set, not _ → Prop, so this is a step in the right direction. The proper fix would be to introduce Language.mk.

Copy link
Member

Choose a reason for hiding this comment

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

Set.up is pretty much exactly that function, as SetSemiring has all the same instances as Language.

Copy link
Member

@eric-wieser eric-wieser left a comment

Choose a reason for hiding this comment

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

I didn't look at the CategoryTheory files but the rest look fine.

@ocfnash
Copy link
Contributor

ocfnash commented Jul 26, 2023

Thanks!

bors merge

@github-actions github-actions bot added ready-to-merge This PR has been sent to bors. and removed awaiting-review The author would like community review of the PR labels Jul 26, 2023
bors bot pushed a commit that referenced this pull request Jul 26, 2023
- Use `{x | p x}` instead of `fun x ↦ p x` to define a set here and there.
- Golf some proofs.
- Replace `Con.ker_apply_eq_preimage` with `Con.ker_apply`. The old version used to abuse definitional equality between `Set M` and `M → Prop`.
- Fix `Submonoid.mk*` lemmas to use `⟨_, _⟩`, not `⟨⟨_, _⟩, _⟩`.
@bors
Copy link

bors bot commented Jul 26, 2023

Pull request successfully merged into master.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot changed the title chore: fix some Set defeq abuse, golf [Merged by Bors] - chore: fix some Set defeq abuse, golf Jul 26, 2023
@bors bors bot closed this Jul 26, 2023
@bors bors bot deleted the YK-setof-defeq branch July 26, 2023 10:47
semorrison pushed a commit that referenced this pull request Aug 14, 2023
- Use `{x | p x}` instead of `fun x ↦ p x` to define a set here and there.
- Golf some proofs.
- Replace `Con.ker_apply_eq_preimage` with `Con.ker_apply`. The old version used to abuse definitional equality between `Set M` and `M → Prop`.
- Fix `Submonoid.mk*` lemmas to use `⟨_, _⟩`, not `⟨⟨_, _⟩, _⟩`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has been sent to bors.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants