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(measure_theory/*): make measurable_space arguments implicit, determined by the measure argument #8571

Closed
wants to merge 17 commits into from

Conversation

RemyDegenne
Copy link
Collaborator

@RemyDegenne RemyDegenne commented Aug 7, 2021

In the measure theory library, most of the definitions that depend on a measure have that form:

def example_def {α} [measurable_space α] (μ : measure α) : some_type := sorry

Suppose now that we have two measurable_space structures on α : {m m0 : measurable_space α} and we have the measures μ : measure α (which is a measure on m0) and μm : @measure α m. This will be common for probability theory applications. See for example the conditional_expectation file.
Then we can write example_def μ but we cannot write example_def μm because the measurable_space inferred is m0, which does not match the measurable space on which μm is defined. We have to use @example_def _ m μm instead.

This PR implements a simple fix: change [measurable_space α] (μ : measure α) into {m : measurable_space α} (μ : measure α).

Advantage: we can now use example_def μm since the measurable_space argument is deduced from the measure argument. This removes many @ in all places where measure.trim is used.

Downsides:

  • I have to write (0 : measure α) instead of 0 in some lemmas.
  • I had to add two apply_instance to find borel_space instances.
  • Whenever a lemma takes an explicit measure argument, we have to write {m : measurable_space α} (μ : measure α) instead of simply (μ : measure α).

Open in Gitpod

@RemyDegenne RemyDegenne added the WIP Work in progress label Aug 7, 2021
@eric-wieser
Copy link
Member

eric-wieser commented Aug 7, 2021

Suppose now that we have two measurable_space structures on α : {m m0 : measurable_space α} .... This will be common for probability theory applications

If having two non-equal typeclass instances on the same type is going to be common, then this feels it is not an appropriate use of typeclasses in the first place. This PR looks like it makes things slightly less painful, but I think there's an underlying issue here worth thinking about more (as a follow-up). Usually the "multiple structures on the same type" problem is solved either with type aliases or replacing typeclasses with structures. It would be worth thinking about whether either of those approaches would work for probability theory.

I have no objections to this PR, it just seems like a good excuse to draw attention to the underlying potential design issue

@RemyDegenne
Copy link
Collaborator Author

Thanks for the comments, Eric. I agree that there is something worth thinking about more here. I did not want to change anything fundamental yet, since having measurable_space as a typeclass works very well for almost everything the the measure_theory folder.

I limited my changes to the following well defined case: a lemma needs a measurable space and a measure, but works only if the measurable_space is the one used to define the measure. Then the change ensures that the measurable_space is given by the measure argument.
It seems to work quite well: I was able to remove many @. With that change, the current way to use measurable_space seems to fit my needs and I am inclined to leaving it as it is now. I'll see if it becomes inconvenient once more involved cases with many measurable_space appear.

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

This looks very very good to me. Can you merge master?

@github-actions github-actions bot removed the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Aug 11, 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.

I like this a lot. This solves the issue of choosing the sigma-algebra in some situations, while not creating any real problem I can see. Of course, it won't solve all the issues (sometimes, we will still need to resort to @-versions), but in many cases this is a net improvement.

There are probably other situations where this strategy can be applied (see the comment on sigma-finite), but to avoid merge conflicts I wouldn't differ this one more than necessary. So I'd say, let's merge this now and improve later.

bors d+

lemma integrable_of_forall_fin_meas_le' {μ : measure α'} (hm : m ≤ m0)
[@sigma_finite _ m (μ.trim hm)] (C : ℝ≥0∞) (hC : C < ∞) {f : α' → E} (hf_meas : ae_measurable f μ)
lemma integrable_of_forall_fin_meas_le' {μ : measure α} (hm : m ≤ m0)
[@sigma_finite _ m (μ.trim hm)] (C : ℝ≥0∞) (hC : C < ∞) {f : α → E} (hf_meas : ae_measurable f μ)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just to be sure: couldn't also sigma_finite benefit from the same treatment as the other ones in this PR?

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 can also be changed. I forgot this one.

@@ -643,7 +650,7 @@ if hf : injective f ∧ ∀ s, measurable_set s → measurable_set (f '' s) then
end
else 0

lemma comap_apply (f : α → β) (hfi : injective f)
lemma comap_apply {m0 : measurable_space α} (f : α → β) (hfi : injective f)
Copy link
Collaborator

Choose a reason for hiding this comment

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

In this one, I don't see how m0 can be found by Lean. Did you want to put the structure on beta instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch. It should indeed be on beta. Thanks!

@bors
Copy link

bors bot commented Aug 11, 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.

@github-actions github-actions bot added delegated The PR author may merge after reviewing final suggestions. and removed awaiting-review The author would like community review of the PR labels Aug 11, 2021
@RemyDegenne
Copy link
Collaborator Author

bors r+

bors bot pushed a commit that referenced this pull request Aug 12, 2021
…termined by the measure argument (#8571)

In the measure theory library, most of the definitions that depend on a measure have that form:
```
def example_def {α} [measurable_space α] (μ : measure α) : some_type := sorry
```
Suppose now that we have two `measurable_space` structures on `α` : `{m m0 : measurable_space α}` and we have the measures `μ : measure α` (which is a measure on `m0`) and `μm : @measure α m`. This will be common for probability theory applications. See for example the `conditional_expectation` file.
Then we can write `example_def μ` but we cannot write `example_def μm` because the `measurable_space` inferred is `m0`, which does not match the measurable space on which `μm` is defined. We have to use `@example_def _ m μm` instead.

This PR implements a simple fix: change `[measurable_space α] (μ : measure α)` into `{m : measurable_space α} (μ : measure α)`.

Advantage: we can now use `example_def μm` since the `measurable_space` argument is deduced from the `measure` argument. This removes many `@` in all places where `measure.trim` is used.

Downsides:
- I have to write `(0 : measure α)` instead of `0` in some lemmas.
- I had to add two `apply_instance` to find `borel_space` instances.
- Whenever a lemma takes an explicit measure argument, we have to write `{m : measurable_space α} (μ : measure α)` instead of simply `(μ : measure α)`.
@bors
Copy link

bors bot commented Aug 12, 2021

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title chore(measure_theory/*): make measurable_space arguments implicit, determined by the measure argument [Merged by Bors] - chore(measure_theory/*): make measurable_space arguments implicit, determined by the measure argument Aug 12, 2021
@bors bors bot closed this Aug 12, 2021
@bors bors bot deleted the implicit_meas branch August 12, 2021 08:30
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants