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

Some convenience functions for WeightedMeasures #43

Merged
merged 7 commits into from
May 5, 2022

Conversation

theogf
Copy link
Collaborator

@theogf theogf commented May 2, 2022

This include having length returning the right thing, defining a fallback logweight for all measures and being able to sample from a weighted measure.

@github-actions
Copy link

github-actions bot commented May 2, 2022

Package name latest stable
MeasureTheory.jl

@github-actions
Copy link

github-actions bot commented May 2, 2022

Package name latest stable
MeasureTheory.jl

@codecov
Copy link

codecov bot commented May 2, 2022

Codecov Report

Merging #43 (6943c6f) into master (fba62dc) will increase coverage by 0.53%.
The diff coverage is 77.77%.

@@            Coverage Diff             @@
##           master      #43      +/-   ##
==========================================
+ Coverage   37.98%   38.51%   +0.53%     
==========================================
  Files          31       31              
  Lines         724      727       +3     
==========================================
+ Hits          275      280       +5     
+ Misses        449      447       -2     
Impacted Files Coverage Δ
src/combinators/smart-constructors.jl 32.20% <33.33%> (ø)
src/combinators/weighted.jl 55.55% <100.00%> (+9.72%) ⬆️
src/rand.jl 50.00% <0.00%> (+25.00%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fba62dc...6943c6f. Read the comment docs.

@github-actions
Copy link

github-actions bot commented May 2, 2022

Package name latest stable
MeasureTheory.jl

@cscherrer
Copy link
Collaborator

Thanks @theogf . Can you help me understand downstream applications of these? My main concern is to be sure we can keep the semantics consistent.

@theogf
Copy link
Collaborator Author

theogf commented May 3, 2022

I think length and rand are pretty self explicit. For logweight it makes sense to me that a Measure which is not weighted should have a weight of 1?
About the use, well it allows to not use API (like length) on weighted measures as well, but more specifically I would use for the sampling on superposed measure I mentioned here

@cscherrer
Copy link
Collaborator

I'm confused, because I don't think of measures as ever having a length, and I don't know what this would mean. Would it be the number of components of a product or superposition? Or the length of a testvalue? In any of these cases, why not just specify that directly?

Kind of the same issue with logweight. The name makes it sounds like it would be intrinsic to the measure itself, but this seems to instead look at the concrete representation. So for example, if logweight is defined for all measures, I'd think

logweight(3 * (Normal() + StudentT(5))) == log(6)

(the log of the mass of the measure)
But if I'm reading this correctly, it would instead give log(3).

If it's going to not be composable, maybe we should call it _logweight, to show it's an internal and very concrete thing?

@theogf
Copy link
Collaborator Author

theogf commented May 3, 2022

I'm confused, because I don't think of measures as ever having a length, and I don't know what this would mean

It probably should not be called length indeed! What I understood implicitly was the dimension of the variable domain.

The name makes it sounds like it would be intrinsic to the measure itself, but this seems to instead look at the concrete representation

I see what you mean, and your example is great. I will change accordingly. But that also means that the current logweight behavior is wrong right?

@cscherrer
Copy link
Collaborator

But that also means that the current logweight behavior is wrong right?

Haha, yeah I guess so. I don't see anywhere we use that, maybe we just drop it?

@github-actions
Copy link

github-actions bot commented May 4, 2022

Package name latest stable
MeasureTheory.jl

@theogf
Copy link
Collaborator Author

theogf commented May 4, 2022

Btw, back to the length, length on a ProductMeasure returns the dimensions of the input, so having length on WeightedMeasure helps passing this information.

@github-actions
Copy link

github-actions bot commented May 4, 2022

Package name latest stable
MeasureTheory.jl

@theogf
Copy link
Collaborator Author

theogf commented May 4, 2022

Ah but it actually just return the number of marginals, not in the input dimensionality.
There should probably be another function for that. In the mean time I will just remove it.

@github-actions
Copy link

github-actions bot commented May 4, 2022

Package name latest stable
MeasureTheory.jl

@theogf
Copy link
Collaborator Author

theogf commented May 5, 2022

@cscherrer Anything else you would like to see changed?

@cscherrer
Copy link
Collaborator

Looking good, thanks @theogf !

@cscherrer cscherrer merged commit 1d1ad53 into master May 5, 2022
@cscherrer cscherrer deleted the tgf/weightedmeasure branch May 5, 2022 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants