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(measure_theory/tactic): add measurability tactic #7756

Closed
wants to merge 25 commits into from

Conversation

RemyDegenne
Copy link
Collaborator

Add a measurability tactic defined in the file measure_theory/tactic.lean, which is heavily inspired from the continuity tactic. It proves goals of the form measurable f, ae_measurable f µ and measurable_set s. Some tests are provided in tests/measurability.lean and the tactic was used to replace a few lines in integration.lean and mean_inequalities.lean.


Open in Gitpod

It is my first tactic and was obtained by iteratively modifying continuity until it worked, so please review it with extra care.

The majority of the changes in the PR, besides adding [measurability] attibutes everywhere, are due to a split of measurable_space.lean and measure_space.lean each into two files.

  • For measurable_space, I extracted measurable_space_def.lean, which contains definitions (up to the definition of a measurable function), but without most of the measurability lemmas.
  • The file outer_measure.lean then depends only on measurable_space_def.lean, and the new measure_space_def.lean defines measure spaces (leaving out most properties) also importing only measurable_space_def but not measurable_space.

The tactic then depends only on the def files, and the [measurability] attribute is available for use in measurable_space.lean.

I could also leave the files as they are and add the measurability attributes in the tactic file. This is already the way I dealt with the measurable_set lemmas since I could not remove those from measurable_space_def because many were needed in pi_system, outer_measure or measure_space_def. I would like comments on the best way to proceed.

@RemyDegenne RemyDegenne added the WIP Work in progress label May 30, 2021
@github-actions github-actions bot added the merge-conflict Please `git merge origin/master` then a bot will remove this label. label May 30, 2021
@github-actions github-actions bot removed the merge-conflict Please `git merge origin/master` then a bot will remove this label. label May 30, 2021
@RemyDegenne RemyDegenne added awaiting-review The author would like community review of the PR and removed WIP Work in progress labels May 30, 2021
Copy link
Collaborator

@sgouezel sgouezel left a comment

Choose a reason for hiding this comment

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

Thanks so much for this!

src/measure_theory/tactic.lean Outdated Show resolved Hide resolved
src/measure_theory/tactic.lean Outdated Show resolved Hide resolved
Copy link
Collaborator

@sgouezel sgouezel left a comment

Choose a reason for hiding this comment

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

This looks very good to me. The file-split makes sense. I'd be grateful if someone more tactic-inclined could have a look at the file measure_theory/tactic.lean (@robertylewis ? @fpvandoorn ?) as this will bitrot quickly if not merged soon.

src/measure_theory/tactic.lean Outdated Show resolved Hide resolved
@@ -0,0 +1,76 @@
/-
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you add in this file a test for measurability?

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 am not sure about how to test that form yet, but I will look at other test files and find something.

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 really don't know how I should test that. measurability? just does show_term { measurability } and prints the term generated by the tactic. Hence if show_term works as intended and measurability succeeds on a test, then measurability? shows a term that succeeds.
I cannot find tests for show_term nor tests for the specific form tidy?. The tests for library_search simply report the expected message in a comment but don't test that what is returned actually match it. And actually the test in test/library_search/ordered_ring.lean does not match.


Mark lemmas with `@[measurability]` to add them to the set of lemmas
used by `measurability`. Note: `to_additive` doesn't know yet how to
copy the attribute to the additive version.
Copy link
Member

Choose a reason for hiding this comment

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

This seems like something that should be fixed before merging

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do you know how I would do that? I would very much like to do it. And then I'll make another PR to do it for the continuity tactic as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do I just add measurability to the list of names line 312 of algebra/group/to_additive.lean? I'll experiment with that file tomorrow and see if I can make it work.

Copy link
Collaborator

Choose a reason for hiding this comment

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

to_additive doesn't do this for continuity either, fwiw

Copy link
Member

Choose a reason for hiding this comment

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

This is easily fixed after #5487 and need not hold up this PR.

@github-actions github-actions bot added the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Jun 2, 2021
@bors
Copy link

bors bot commented Jun 7, 2021

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

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+


Mark lemmas with `@[measurability]` to add them to the set of lemmas
used by `measurability`. Note: `to_additive` doesn't know yet how to
copy the attribute to the additive version.
Copy link
Member

Choose a reason for hiding this comment

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

This is easily fixed after #5487 and need not hold up this PR.

@fpvandoorn fpvandoorn 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 Jun 8, 2021
@github-actions github-actions bot removed the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Jun 8, 2021
@RemyDegenne
Copy link
Collaborator Author

bors r+

bors bot pushed a commit that referenced this pull request Jun 9, 2021
Add a measurability tactic defined in the file `measure_theory/tactic.lean`, which is heavily inspired from the continuity tactic. It proves goals of the form `measurable f`, `ae_measurable f µ` and `measurable_set s`. Some tests are provided in `tests/measurability.lean` and the tactic was used to replace a few lines in `integration.lean` and `mean_inequalities.lean`.
@bors
Copy link

bors bot commented Jun 9, 2021

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Jun 9, 2021
Add a measurability tactic defined in the file `measure_theory/tactic.lean`, which is heavily inspired from the continuity tactic. It proves goals of the form `measurable f`, `ae_measurable f µ` and `measurable_set s`. Some tests are provided in `tests/measurability.lean` and the tactic was used to replace a few lines in `integration.lean` and `mean_inequalities.lean`.
@bors
Copy link

bors bot commented Jun 10, 2021

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(measure_theory/tactic): add measurability tactic [Merged by Bors] - feat(measure_theory/tactic): add measurability tactic Jun 10, 2021
@bors bors bot closed this Jun 10, 2021
@bors bors bot deleted the measurability_tactic branch June 10, 2021 01:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-author A reviewer has asked the author a question or requested changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants