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

chore(algebra/to_interval_mod): refactor add_comm_group.modeq to have a simpler definition #18941

Closed
wants to merge 12 commits into from

Conversation

eric-wieser
Copy link
Member

@eric-wieser eric-wieser commented May 4, 2023

This redefines it as ∃ z : ℤ, b = a + z • p instead of ∀ z : ℤ, b - z • p ∉ set.Ioo a (a + p) (as requested by @alreadydone), which needs less in the way of typeclass assumptions


Open in Gitpod

eric-wieser and others added 8 commits May 2, 2023 08:07
…m_Ioo_mod`

This replaces `mem_Ioo_mod hp a b` with `¬Imodeq p a b`, where `Imodeq` stands for `interval mod equal`.
This is more consistent with `int.modeq`, `nat.modeq`, and `smodeq`.

There's still some duplication here, but at least these four ideas are now conceptually aligned.

This remove any lemmas of the form `¬Imodeq p a b ↔ _ ≠ _` as these are now trivial consequences of the `Imodeq p a b ↔ _ = _` versions,
Co-authored-by: Yaël Dillies <yael.dillies@gmail.com>
Co-authored-by: Junyan Xu <junyanxumath@gmail.com>
@eric-wieser eric-wieser added the awaiting-review The author would like community review of the PR label May 4, 2023
Comment on lines +69 to +71
lemma not_modeq_iff_ne_mod_zmultiples :
¬a ≡ b [PMOD p] ↔ (b : α ⧸ add_subgroup.zmultiples p) ≠ a :=
modeq_iff_eq_mod_zmultiples.not
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wouldn't mind if you remove this lemma.

src/algebra/order/to_interval_mod.lean Show resolved Hide resolved
Equivalently (as shown further down the file), `b` does not lie in the open interval `(a, a + p)`
modulo `p`, or `to_Ico_mod hp a` disagrees with `to_Ioc_mod hp a` at `b`, or `to_Ico_div hp a`
disagrees with `to_Ioc_div hp a` at `b`. -/
def modeq (p a b : α) : Prop := ∃ z : ℤ, b = a + z • p
Copy link
Collaborator

Choose a reason for hiding this comment

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

This definition is probably easiest to use, but would be a departure from nat.modeq, int.modeq, and smodeq which are defined to be equality after the mod (%) operation or equality in the quotient. Is this what @YaelDillies plan to use as the general definition?

Copy link
Collaborator

Choose a reason for hiding this comment

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

People have complained in the past that int.modeq was defined using % and that nat.modeq wasn't defined in terms of int.modeq. My plan of action is to

  1. define add_comm_group.modeq. Easy
  2. drop int.modeq in favour of it. Probably easy, just have to realign some names and fix a few proofs (most proofs don't use the definition of int.modeq because it is terrible)
  3. redefine nat.modeq in terms of it. Easy.
  4. replace add_comm_group.modeq with notation around smodeq. Potentially hard because add_comm_group.modeq is the special case of smodeq where the submodule is generated by a single element.

It's likely we can only do step 1 before the port. Also note that if we go through all those steps, the definition of int.modeq will change twice, and hopefully that's okay because as said above almost no proof relies on the defeq because of how useless it is.

Note however that my new definition of add_comm_group.modeq isn't quite the same as the one Eric has here. I instead have

def modeq (p a b : α) : Prop := ∃ z : ℤ, b - a = z • p

which is defeq to p | b - a when p a b : ℤ. You can see my progress on the branch eric-wieser/redef-Imodeq.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This sounds like a good plan, and your add_comm_group.modeq is also defeq to the proposed new smodeq definition applied to zmultiples, so maybe step 4 won't be hard once we adopt the new smodeq.

For future reference, let me note that smodeq (and add_comm_group.modeq) could be generalized further from add_comm_group to add_group with exactly the same definition, and then it would mean that two elements lie in the same right_coset (to express that two elements lie in the same left coset, we may say their inverses lie in the same right coset), and it could also be to_additive'd from the multiplicative version.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh indeed! Then if @eric-wieser agrees, I suggest we merge eric-wieser/redef-Imodeq into this PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

It sounds like you both have stronger opinions on this than me. I propose we either:

  1. Merge this PR as is, and let @YaelDillies create a follow up from the branch above
  2. Abandon this PR and let @YaelDillies do the whole thing directly from the branch above
  3. Abandon this PR and refrain from doing anything until mathlib4

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm happy to go with 2 if you're tired of leading the charge.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

@eric-wieser @YaelDillies Does that mean that this PR should now be closed?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, let's close it.

@mathlib-dependent-issues-bot mathlib-dependent-issues-bot added the blocked-by-other-PR This PR depends on another PR which is still in the queue. A bot manages this label via PR comment. label May 6, 2023
bors bot pushed a commit that referenced this pull request May 7, 2023
@mathlib-dependent-issues-bot mathlib-dependent-issues-bot removed the blocked-by-other-PR This PR depends on another PR which is still in the queue. A bot manages this label via PR comment. label May 7, 2023
@mathlib-dependent-issues-bot
Copy link
Collaborator

@YaelDillies
Copy link
Collaborator

Subsumed by #18958

@YaelDillies YaelDillies closed this May 8, 2023
@YaelDillies YaelDillies deleted the eric-wieser/redef-add_comm_group.modeq branch May 8, 2023 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-review The author would like community review of the PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants