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: Laws for monads with Alternative instances. #1152

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

dtumad
Copy link

@dtumad dtumad commented Mar 3, 2025

This PR adds type-classes AlternativeMonad, LawfulAlternative, and LawfulAlternativeLift that enforce laws regarding failure and orElse.

Lemmas about OptionT are also upstreamed from mathlib to use in the LawfulAlternative proof. OptionT.run_bind was changed as part of this move to use Option.elimM rather than explicit pattern matching in the lemma.

Additional instances for Set and List need to be written to mathlib as that is where the main monad instances for those types are.

@github-actions github-actions bot added the awaiting-review This PR is ready for review; the author thinks it is ready to be merged. label Mar 3, 2025
@leanprover-community-bot
Copy link
Collaborator

leanprover-community-bot commented Mar 5, 2025

Mathlib CI status (docs):

@Rob23oba
Copy link
Contributor

Rob23oba commented Mar 6, 2025

I wouldn't immediately go for defining LawfulAlternative on AlternativeMonads, the failure_bind rule can be recovered from map_failure and failure_seq:

class LawfulAlternative (m : Type u → Type v) [Alternative m] extends LawfulApplicative m where
  map_failure {α β : Type u} (f : α → β) : f <$> (failure : m α) = failure
  failure_seq {α β : Type u} (x : m α) : (failure : m (α → β)) <*> x = failure
  -- ...

export LawfulAlternative (map_failure failure_seq)

class AlternativeMonad (m : Type u → Type v) extends Alternative m, Monad m

theorem failure_bind [AlternativeMonad m] [LawfulMonad m] [LawfulAlternative m] (x : α → m β) : failure >>= x = failure := by
  conv => lhs; rw [← map_failure PEmpty.elim]
  conv => rhs; rw [← bind_pure failure]
  conv => rhs; rw [← map_failure PEmpty.elim]
  simp only [bind_map_left]
  congr 1; funext a
  exact a.elim

@dtumad
Copy link
Author

dtumad commented Mar 6, 2025

@Rob23oba updated to that definition for the class, using PEmpty.elim in the proof like that is a very neat trick.

@dtumad
Copy link
Author

dtumad commented Mar 6, 2025

What's the procedure for updating mathlib after upstreaming these OptionT lemmas? I'm not sure how to handle coordinating the two changes, should I just open a mathlib PR with the relevant lemmas removed? Or remove the imports from Batteries.lean so that mathlib doesn't see the duplicate lemmas when building mathlib?

Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
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.

LGTM, thanks!

You should get a mathlib adaption PR ready to go before this gets merged, so as not to hold up merging other PRs.

leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Mar 7, 2025
@fgdorais
Copy link
Collaborator

This needs a Mathlib adaptation before merging. Ideally, this just involves pushing changes to the Mathlib branch batteries-pr-testing-1152 until it works. Let us know if you have questions of if you run into complications.

@fgdorais
Copy link
Collaborator

Thanks! Could you create an actual Mathlib PR out of batteries-pr-testing-1152. I think this needs a review on the Mathlib end. You may have to ping someone to review it. Ask @eric-wieser if they are interested or if they have alternate recommendations for that review. Once that's done, ping back here since Batteries maintainers won't be notified automatically.

@eric-wieser
Copy link
Member

I'd be happy to review it

@dtumad
Copy link
Author

dtumad commented Mar 11, 2025

Sorry about the slow response. This PR has the needed changes. Should I change the mathlib lakefile back to main batteries repo before this gets merged here?

@eric-wieser
Copy link
Member

@fgdorais, I'm happy with the adaptation PR, just the stray comment to delete above.

@fgdorais
Copy link
Collaborator

I made a few style changes because Batteries prefers to avoid section variables and also makes systematic use of autoimplicits. Please have a quick look at my changes in case there are instances where Lean doesn't automatically make the right choice.

leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Mar 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-review This PR is ready for review; the author thinks it is ready to be merged. builds-mathlib
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants