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] - chore: add some simp and fun_prop attributes #18874

Closed
wants to merge 5 commits into from

Conversation

fpvandoorn
Copy link
Member

@fpvandoorn fpvandoorn commented Nov 11, 2024

  • Also add a test file that uses the simp lemmas.
  • Add/move the fun_prop lemmas to the compositional lemmas.

Zulip


Open in Gitpod

Copy link

github-actions bot commented Nov 11, 2024

PR summary 7456389336

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-analysis Analysis (normed *, calculus) label Nov 11, 2024
Copy link
Collaborator

@grunweg grunweg left a comment

Choose a reason for hiding this comment

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

Looks innocuous, and useful enough. Two nits; please address before merging. Thanks!
maintainer delegate

Copy link

🚀 Pull request has been placed on the maintainer queue by grunweg.

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.

It turns out that these are not actually the most useful lemmas to tag with fun_prop in practice (although doing these doesn't really hurt; I know mainly what you are adding is simp. What you actually want are the compositional forms of all of these lemmas. That is: we actually want things like Differentiable.exp to be marked with fun_prop. While we're here do you mind marking those accordingly?

@j-loreaux j-loreaux added awaiting-author A reviewer has asked the author a question or requested changes and removed maintainer-merge labels Nov 11, 2024
@fpvandoorn
Copy link
Member Author

It turns out that these are not actually the most useful lemmas to tag with fun_prop in practice (although doing these doesn't really hurt; I know mainly what you are adding is simp. What you actually want are the compositional forms of all of these lemmas. That is: we actually want things like Differentiable.exp to be marked with fun_prop. While we're here do you mind marking those accordingly?

I've noticed that the number of failures of fun_prop actually increases when tracing its output, so I expect it is important not to tag them both for performance reasons.

I applied the fun_prop attributes to the compositional forms only, and therefore I removed some fun_prop attributes in Trigonometric.Deriv.

@fpvandoorn fpvandoorn requested a review from j-loreaux November 14, 2024 10:40
@fpvandoorn fpvandoorn removed the awaiting-author A reviewer has asked the author a question or requested changes label Nov 18, 2024
@j-loreaux
Copy link
Collaborator

bors merge

@github-actions github-actions bot added the ready-to-merge This PR has been sent to bors. label Nov 18, 2024
mathlib-bors bot pushed a commit that referenced this pull request Nov 18, 2024
* Also add a test file that uses the `simp` lemmas.
* Add/move the `fun_prop` lemmas to the compositional lemmas.

[Zulip](https://leanprover.zulipchat.com/#narrow/channel/287929-mathlib4/topic/Derivative.20cannot.20simp)
@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Nov 18, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title chore: add some simp and fun_prop attributes [Merged by Bors] - chore: add some simp and fun_prop attributes Nov 18, 2024
@mathlib-bors mathlib-bors bot closed this Nov 18, 2024
@mathlib-bors mathlib-bors bot deleted the simp-deriv branch November 18, 2024 15:35
TobiasLeichtfried pushed a commit that referenced this pull request Nov 21, 2024
* Also add a test file that uses the `simp` lemmas.
* Add/move the `fun_prop` lemmas to the compositional lemmas.

[Zulip](https://leanprover.zulipchat.com/#narrow/channel/287929-mathlib4/topic/Derivative.20cannot.20simp)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has been sent to bors. t-analysis Analysis (normed *, calculus)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants