Skip to content

Conversation

@YaelDillies
Copy link
Collaborator

@YaelDillies YaelDillies commented Mar 18, 2024

Among other things, change the nsmul, zsmul, qsmul fields to have n/q come before x, because this matches the lemmas we want to write about them. It would be preferrable to perform the same changes to the AddMonoid/AddGroup-like typeclasses, but this is impossible with the current to_additive framework, so instead I have inserted some Function.swap at the interface between AddMonoid/AddGroup and Ring/Field.

Reduce the diff of #11203


Open in Gitpod

@YaelDillies YaelDillies added awaiting-review t-algebra Algebra (groups, rings, fields, etc) t-order Order theory labels Mar 18, 2024
(neg : ∀ x, f (-x) = -f x) (sub : ∀ x y, f (x - y) = f x - f y) (inv : ∀ x, f x⁻¹ = (f x)⁻¹)
(div : ∀ x y, f (x / y) = f x / f y)
(nsmul : ∀ (x) (n : ℕ), f (n • x) = n • f x)
(zsmul : ∀ (x) (n : ℤ), f (n • x) = n • f x) (qsmul : ∀ (q : ℚ) (x), f (q • x) = q • f x)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Did you intentionally change the order of arguments to qsmul to no longer match nsmul and zsmul?

Copy link
Collaborator Author

@YaelDillies YaelDillies Mar 18, 2024

Choose a reason for hiding this comment

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

Yes, this is so that lemmas about the coercion qsmul can be used without shuffling their arguments.

Copy link
Member

@eric-wieser eric-wieser Mar 18, 2024

Choose a reason for hiding this comment

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

Please add a comment to the docstring of this file noting the rationale for the inconsistent ordering of the binders in the nsmul and zsmul arguments.

@eric-wieser eric-wieser added awaiting-author A reviewer has asked the author a question or requested changes. and removed awaiting-review labels Mar 19, 2024
@YaelDillies YaelDillies added awaiting-review and removed awaiting-author A reviewer has asked the author a question or requested changes. labels Mar 19, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Apr 5, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Apr 5, 2024
Copy link
Member

@fpvandoorn fpvandoorn left a comment

Choose a reason for hiding this comment

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

LGTM

bors d+

-- Porting Note: Commented
-- attribute [local semireducible] locallyRingedSpaceAdjunction ΓSpec.adjunction

set_option maxHeartbeats 400000 in
Copy link
Member

Choose a reason for hiding this comment

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

any idea what changed here / how to make this better / avoid the convert?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The proof uses 200959 heartbeats and basically the same on master so I suspect this is just noise

@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Apr 5, 2024

✌️ YaelDillies can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added delegated This pull request has been delegated to the PR author (or occasionally another non-maintainer). and removed awaiting-review labels Apr 5, 2024
@YaelDillies
Copy link
Collaborator Author

bors merge

mathlib-bors bot pushed a commit that referenced this pull request Apr 5, 2024
Among other things, change the `nsmul`, `zsmul`, `qsmul` fields to have `n`/`q` come before `x`, because this matches the lemmas we want to write about them. It would be preferrable to perform the same changes to the `AddMonoid`/`AddGroup`-like typeclasses, but this is impossible with the current `to_additive` framework, so instead I have inserted some `Function.swap` at the interface between `AddMonoid`/`AddGroup` and `Ring`/`Field`.

Reduce the diff of #11203
@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Apr 5, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title chore(Field/InjSurj): Tidy [Merged by Bors] - chore(Field/InjSurj): Tidy Apr 5, 2024
@mathlib-bors mathlib-bors bot closed this Apr 5, 2024
@mathlib-bors mathlib-bors bot deleted the cleanup_field_inj_surj branch April 5, 2024 23:02
xgenereux pushed a commit that referenced this pull request Apr 15, 2024
Among other things, change the `nsmul`, `zsmul`, `qsmul` fields to have `n`/`q` come before `x`, because this matches the lemmas we want to write about them. It would be preferrable to perform the same changes to the `AddMonoid`/`AddGroup`-like typeclasses, but this is impossible with the current `to_additive` framework, so instead I have inserted some `Function.swap` at the interface between `AddMonoid`/`AddGroup` and `Ring`/`Field`.

Reduce the diff of #11203
atarnoam pushed a commit that referenced this pull request Apr 16, 2024
Among other things, change the `nsmul`, `zsmul`, `qsmul` fields to have `n`/`q` come before `x`, because this matches the lemmas we want to write about them. It would be preferrable to perform the same changes to the `AddMonoid`/`AddGroup`-like typeclasses, but this is impossible with the current `to_additive` framework, so instead I have inserted some `Function.swap` at the interface between `AddMonoid`/`AddGroup` and `Ring`/`Field`.

Reduce the diff of #11203
uniwuni pushed a commit that referenced this pull request Apr 19, 2024
Among other things, change the `nsmul`, `zsmul`, `qsmul` fields to have `n`/`q` come before `x`, because this matches the lemmas we want to write about them. It would be preferrable to perform the same changes to the `AddMonoid`/`AddGroup`-like typeclasses, but this is impossible with the current `to_additive` framework, so instead I have inserted some `Function.swap` at the interface between `AddMonoid`/`AddGroup` and `Ring`/`Field`.

Reduce the diff of #11203
callesonne pushed a commit that referenced this pull request Apr 22, 2024
Among other things, change the `nsmul`, `zsmul`, `qsmul` fields to have `n`/`q` come before `x`, because this matches the lemmas we want to write about them. It would be preferrable to perform the same changes to the `AddMonoid`/`AddGroup`-like typeclasses, but this is impossible with the current `to_additive` framework, so instead I have inserted some `Function.swap` at the interface between `AddMonoid`/`AddGroup` and `Ring`/`Field`.

Reduce the diff of #11203
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

delegated This pull request has been delegated to the PR author (or occasionally another non-maintainer). t-algebra Algebra (groups, rings, fields, etc) t-order Order theory

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants