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] - refactor: Let positivity handle ENNReal-valued ofNat #17212

Closed
wants to merge 5 commits into from

Conversation

YaelDillies
Copy link
Collaborator

@YaelDillies YaelDillies commented Sep 28, 2024

The meta code was looking for StrictOrderedSemiring instead of the weaker OrderedSemiring + Nontrivial, which is enough.

From LeanAPAP


Open in Gitpod

@YaelDillies YaelDillies added the t-meta Tactics, attributes or user commands label Sep 28, 2024
Copy link

github-actions bot commented Sep 28, 2024

PR summary 8d8c7cc2c8

Import changes for modified files

No significant changes to the import graph

Import changes for all files
Files Import difference

Declarations diff

No declarations were harmed in the making of this PR! 🐙

You can run this locally as follows
## summary with just the declaration names:
./scripts/declarations_diff.sh <optional_commit>

## more verbose report:
./scripts/declarations_diff.sh long <optional_commit>

The doc-module for script/declarations_diff.sh contains some details about this script.

@github-actions github-actions bot added the t-data Data (lists, quotients, numbers, etc) label Sep 28, 2024
Mathlib/Data/ENNReal/Basic.lean Outdated Show resolved Hide resolved
Comment on lines 732 to 733
example : (0 : ℝ≥0∞) < 1 := by positivity
example : (0 : ℝ≥0∞) < 2 := by positivity
Copy link
Collaborator

Choose a reason for hiding this comment

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

I might add a test for \le or \ne as well, but I'll leave it to you whether that's useful

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No indeed, positivity already takes care of that

@hrmacbeth
Copy link
Member

hrmacbeth commented Oct 3, 2024

Rather than having a special ENNReal-ofNat positivity extension, I think we should fix the existing positivity extension for numerals to handle this use case. Note that currently

import Mathlib

example : 0 < (2:ENNReal) := by positivity -- fails
example : 0 ≤ (2:ENNReal) := by positivity -- works

The reason is that in the tactic code proving requires OrderedSemiring (which holds for ENNReal) but proving < requires StrictOrderedSemiring (which does not hold for ENNReal).

@YaelDillies I am sure you are very capable of finding or inventing a typeclass which (i) holds for ENNReal and (ii) implies that positive numerals are positive.

@hrmacbeth hrmacbeth added the awaiting-author A reviewer has asked the author a question or requested changes label Oct 3, 2024
@YaelDillies
Copy link
Collaborator Author

Well, that was easy. For some reason I remembered that Nat.cast_pos had to use StrictOrderedSemiring, but clearly I was wrong and short-memoried since no one else than me generalised Nat.cast_pos back in leanprover-community/mathlib3#16172.

@YaelDillies YaelDillies removed the awaiting-author A reviewer has asked the author a question or requested changes label Oct 3, 2024
@YaelDillies YaelDillies changed the title feat: positivity extension for ENNReal-valued ofNat refactor: Let positivity handle ENNReal-valued ofNat Oct 3, 2024
@@ -23,6 +23,9 @@ example : 0 ≤ 3 := by positivity

example : 0 < 3 := by positivity

example : (0 : ℝ≥0∞) < 1 := by positivity
example : (0 : ℝ≥0∞) < 2 := by positivity
Copy link
Member

@hrmacbeth hrmacbeth Oct 3, 2024

Choose a reason for hiding this comment

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

Could you please add a test like this?

example : (0 : ℝ≥0∞) < 3 - 2 := by positivity

(since the norm_num positivity extension should handle that too.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It doesn't work. I suspect it's because norm_num doesn't know about truncated subtraction.

Is your "should" meant as in "it should work" or as in "you should make it work"? I think it's pretty out of scope for this PR regardless of your answer.

In fact, my motivation here is to have positivity prove that the L2 norm of a nonzero function is positive, and there I really don't have any truncated subtraction

Copy link
Member

Choose a reason for hiding this comment

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

I was operating under the mistaken assumption that norm_num reduces (2:ENNReal) - 1 to 1. (It does for Nat but apparently not here.). No action required!

@hrmacbeth
Copy link
Member

bors d+

@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Oct 3, 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.

@YaelDillies
Copy link
Collaborator Author

YaelDillies commented Oct 3, 2024

I will try to make the 3 - 2 example work in another PR. Let's keep this one simple.

bors merge

@YaelDillies
Copy link
Collaborator Author

bors merge

mathlib-bors bot pushed a commit that referenced this pull request Oct 3, 2024
The meta code was looking for `StrictOrderedSemiring` instead of the weaker `OrderedSemiring` + `Nontrivial`, which is enough.

From LeanAPAP
@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Oct 3, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title refactor: Let positivity handle ENNReal-valued ofNat [Merged by Bors] - refactor: Let positivity handle ENNReal-valued ofNat Oct 3, 2024
@mathlib-bors mathlib-bors bot closed this Oct 3, 2024
@mathlib-bors mathlib-bors bot deleted the positivity_of_nat_ennreal branch October 3, 2024 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
delegated t-data Data (lists, quotients, numbers, etc) t-meta Tactics, attributes or user commands
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants