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] - feat(analysis/{normed/group}/seminorm): Hom classes for seminorms #16227

Closed
wants to merge 12 commits into from

Conversation

YaelDillies
Copy link
Collaborator

@YaelDillies YaelDillies commented Aug 24, 2022

Introduce add_group_seminorm_class, group_seminorm_class, seminorm_class, the hom classes of add_group_seminorm, group_seminorm, seminorm.


Open in Gitpod

@YaelDillies YaelDillies added awaiting-review The author would like community review of the PR t-analysis Analysis (normed *, calculus) labels Aug 24, 2022
@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 Aug 24, 2022
@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 Aug 24, 2022
@mathlib-dependent-issues-bot
Copy link
Collaborator

@Vierkantor Vierkantor self-assigned this Aug 30, 2022
Copy link
Collaborator

@Vierkantor Vierkantor left a comment

Choose a reason for hiding this comment

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

I think this looks good overall but someone who knows about seminorms should definitely check that nothing weird is happening.

src/analysis/normed/group/seminorm.lean Show resolved Hide resolved
src/analysis/normed/group/seminorm.lean Show resolved Hide resolved
src/analysis/seminorm.lean Show resolved Hide resolved
@Vierkantor Vierkantor added awaiting-author A reviewer has asked the author a question or requested changes and removed awaiting-review The author would like community review of the PR labels Aug 30, 2022
Co-authored-by: Anne Baanen <Vierkantor@users.noreply.github.com>
@YaelDillies YaelDillies added awaiting-review The author would like community review of the PR and removed awaiting-author A reviewer has asked the author a question or requested changes labels Aug 30, 2022
Copy link
Collaborator

@Vierkantor Vierkantor left a comment

Choose a reason for hiding this comment

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

Thanks!

bors r+

src/analysis/normed/group/seminorm.lean Show resolved Hide resolved
@leanprover-community-bot-assistant leanprover-community-bot-assistant added ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) and removed awaiting-review The author would like community review of the PR labels Sep 2, 2022
@Vierkantor
Copy link
Collaborator

Sorry, I thought this was already reviewed by someone else.

bors r-

bors d=@eric-wieser

@bors
Copy link

bors bot commented Sep 2, 2022

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

@bors
Copy link

bors bot commented Sep 2, 2022

Canceled.

@Vierkantor Vierkantor added delegated The PR author may merge after reviewing final suggestions. and removed ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) labels Sep 2, 2022
@Vierkantor Vierkantor removed their assignment Sep 2, 2022
@Vierkantor
Copy link
Collaborator

(Please feel free to re-assign to someone else, I just picked the first suggestion by GitHub)

@YaelDillies
Copy link
Collaborator Author

I think Eric is still on holidays.

@Vierkantor
Copy link
Collaborator

(Please feel free to re-assign to someone else, I just picked the first suggestion by GitHub)

:)

map_mul := λ f, f.map_mul' }
map_zero := λ f, (f.eq_zero' _).2 rfl }

instance mul_hom_class : mul_hom_class (absolute_value R S) R S :=
Copy link
Collaborator

Choose a reason for hiding this comment

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

As @Vierkantor comments for add_group_seminorm_class.to_zero_hom_class, wouldn't it be good to make coe explicit in this and the following instances?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agreed, copying the data carrying fields is helpful.

@mariainesdff
Copy link
Collaborator

Besides my comment above, it looks good to me.

Copy link
Collaborator

@j-loreaux j-loreaux left a comment

Choose a reason for hiding this comment

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

Despite having very few comments, I have tried to look at this carefully, and it looks good to me.

/-- `subadditive_hom_class F α β` states that `F` is a type of subadditive morphisms. -/
class subadditive_hom_class (F : Type*) (α β : out_param $ Type*) [has_add α] [has_add β] [has_le β]
extends fun_like F α (λ _, β) :=
(map_add_le_add (f : F) : ∀ a b, f (a + b) ≤ f a + f b)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is just bikeshedding, but I think it's reasonable to shorten this to map_add_le.

Suggested change
(map_add_le_add (f : F) : ∀ a b, f (a + b) ≤ f a + f b)
(map_add_le (f : F) : ∀ a b, f (a + b) ≤ f a + f b)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I kept them long to avoid ambiguity with map_mul_le_add (and the eventual map_add_le_max and such).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Feel free to shorten them if we see that no ambiguity arises over time.

bors merge

@[to_additive subadditive_hom_class]
class submultiplicative_hom_class (F : Type*) (α β : out_param $ Type*) [has_mul α] [has_mul β]
[has_le β] extends fun_like F α (λ _, β) :=
(map_mul_le_mul (f : F) : ∀ a b, f (a * b) ≤ f a * f b)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here

Suggested change
(map_mul_le_mul (f : F) : ∀ a b, f (a * b) ≤ f a * f b)
(map_mul_le (f : F) : ∀ a b, f (a * b) ≤ f a * f b)

@jcommelin
Copy link
Member

Thanks, LGTM

bors d+

@bors
Copy link

bors bot commented Sep 9, 2022

✌️ 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.

@github-actions github-actions bot removed the awaiting-review The author would like community review of the PR label Sep 9, 2022
bors bot pushed a commit that referenced this pull request Sep 9, 2022
…6227)

Introduce `add_group_seminorm_class`, `group_seminorm_class`, `seminorm_class`, the hom classes of `add_group_seminorm`, `group_seminorm`, `seminorm`.
@bors
Copy link

bors bot commented Sep 9, 2022

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(analysis/{normed/group}/seminorm): Hom classes for seminorms [Merged by Bors] - feat(analysis/{normed/group}/seminorm): Hom classes for seminorms Sep 9, 2022
@bors bors bot closed this Sep 9, 2022
@bors bors bot deleted the seminorm_class branch September 9, 2022 14:44
bottine pushed a commit that referenced this pull request Sep 13, 2022
…6227)

Introduce `add_group_seminorm_class`, `group_seminorm_class`, `seminorm_class`, the hom classes of `add_group_seminorm`, `group_seminorm`, `seminorm`.
b-mehta pushed a commit that referenced this pull request Sep 21, 2022
…6227)

Introduce `add_group_seminorm_class`, `group_seminorm_class`, `seminorm_class`, the hom classes of `add_group_seminorm`, `group_seminorm`, `seminorm`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
delegated The PR author may merge after reviewing final suggestions. t-analysis Analysis (normed *, calculus)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants