-
Notifications
You must be signed in to change notification settings - Fork 4
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
Changes for MeasureBase v0.15 #122
Draft
oschulz
wants to merge
28
commits into
main
Choose a base branch
from
mt-015
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #122 +/- ##
==========================================
+ Coverage 54.05% 55.74% +1.69%
==========================================
Files 43 41 -2
Lines 1221 1175 -46
==========================================
- Hits 660 655 -5
+ Misses 561 520 -41
☔ View full report in Codecov by Sentry. |
Closed
Draft
Will be used a lot when bridging from Distributions to MeasureBase.
Unused and untested.
Not used currently.
pullback has a huge potential for naming conflickts, and pullbck is more in line with pushfwd. Also simplify implementation of pullbck.
Bind has too much naming conflict potential with Base.bind. The rightarrowtail operator looks very similar to the `>=>` "fish" operator (e.g. in Haskell), which is not a monadic bind.
Removes the integral operators from MeasureBase, to be re-introduced in the submodule MeasureOperators. Also improves the likelihood documentation.
A rebase can easily be written explicitly.
To be re-introduced in sub-module MeasureOperators.
`mintegral` should be used instead to express posteriors.
To be reintroduced in submodule MeasureOperators
Absolute continuity is not really implemented yet.
Having the operators in a sub-module makes it easier for users to control whether of they want them in their namespace. Operators have a larger naming conflict potential.
Co-authored-by: Chad Scherrer <[email protected]>
Co-authored-by: Chad Scherrer <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is collects changes and improvements that will become MeasureBase v0.15, hopfully early next week (there are some deadlines I need to meet). They go through individual PR's and are collected here for general comments before releasing v0.15.
CC @mschauer, @theogf
Merged pull requests: #119, #121
Major changes so far:
The
≪
ll-operator has been removed. There isn't an actual implementation of absolute continuity yet. We can bring this back later, one we have that.PointwiseProductMeasure
has been removed. It was basically the same as integration, but was described in a way limited to building posteriors.New exported functions
mintegrate
andmintegrate_exp
have been added to have function names to go with the existing∫
and∫exp
operators.The field order in
Bind
was changed to have the function first, as is common Julia practice.bind
(not exported) was renamed tombind
(exported).bind
conflicts withBase.bind
which has completely different semantics, and should not be used even as a non-exported function.mbind
is "free" in the Julia ecosystem, and so can and should be exported, as it is a very common operation we need.The
rebase
function was removed. It wasn't really used and one can just writemintegrate(density_rel(μ, ν), ν)
directly, which is more intuitive to read.The
↣
operator used formbind
(formerlybind
) looks very similar to the>=>
"fish" operator in functional programming (e.g. in Haskell), which is not a monadic bind but a concatenation of kernel functions. This is confusing, so we changed the operator to⊛
(which is also used in Haskell for monadic bind). Me and @cscherrer did an extensive search for suitable alternatives and settled on the▷
as a replacement.The function
pullback
was renamed topullbck
which now exported.pullback
is heavily used for AD already, andpullbck
matches the existingpushfwd
. * We use⋄
as the pushforward operator and⊙
as the pullback operator now.The function
kernelfactor
was removed. It was undocumented and not used at all, so it wasn't even clear if it was part of the public API.PowerWeightedMeasure
and it's operator↑
were removed. It wasn't used in MeasureBase or MeasureTheory, and we couldn't find use of it in other packages. It was also undocmented and untested and the semantics are not entirely clear, since the resulting measure seems to depend on an assumed implicit reference measure.@cscherrer: I like the idea of
PowerWeightedMeasure
, and @mschauer has found some good use for it for sampling-based likelihood approximation. But beyond the lack of tests and documentation, there's a fundamental problem we need to straighten out. The idea here was to construct a new measure by taking some existing measure's density to some power. But as always, there's the question density with respect to what? This construction is probably more reliably expressed as a power of a likelihood (which, together with a prior, can comprise a new measure). So we'll remove this code.@mschauer, if you need this we'll keep it, of course (though if it's not a super-frequent operation we should still remove the operator and choose a function name for it).
@cscherrer: We removed some code that in v0.14 allowing constraints when constucting a likelihood from a parameterized measure. I don't think anyone was using these, but I think @nignatiadis had expressed some interest. But what we had was not very expressive, and also didn't compose well. Nikos, I think you had mentioned some ideas about this - if you like, we can discuss this some more in a new issue.
A lot of docstring were improved, esp. on likelihoods, but also in other places.
All measure operators (
⋄
,⊙
,▷
,⊗
,∫
,∫exp
,𝒹
andlog𝒹
) have been moved to the submoduleMeasureOperators
. Operators have a high name conflict potential, this gives the user an easy way to decide of they want them in their namespace or not.There are currently not deprecations in regard to the breaking changes, it seemed to to @cscherrer and me to make a clean cut. If you need specific deprecations, please let us know.
Still to do (should be ready very soon)
Add HierarchicalMeasure #120: Add a hierarchical measure type that turns a measure and a kernel into a joint measure. In the Bayesian case it turns the prior and the likelihood kernel into the joint measure over parameters and observations. It's also necessary to express hierarchical measures (esp. priors).
Finish transport implementation for procucts.
Add a Pkg extension that takes over the role of DistributionMeasures. That way, support for Distributions will become active automatically when Distributions is loaded.