-
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
Dev #49
Conversation
Codecov Report
@@ Coverage Diff @@
## master #49 +/- ##
==========================================
+ Coverage 38.51% 39.80% +1.28%
==========================================
Files 31 30 -1
Lines 727 706 -21
==========================================
+ Hits 280 281 +1
+ Misses 447 425 -22
Continue to review full report at Codecov.
|
Hmm, tests passing on 1.8 and nightly, but not 1.6 or 1.7... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not look at everything in detail (this is a massive PR). But generally, it looks like many tests are missing, I think I caught some wrong implementations which would have been caught with some unit tests. At some point, I would be happy to help with the reorganization of the tests for the package.
I would not delete completely kleisli
, it's still a nice thing to know and learn, and it can be exported. Maybe just leave it as an alias to kernel
? It's just my opinion though.
Kernel/Kleisli contention
We originally had
Kernel
. But this is very overloaded across the ecosystem. Also, what we have isn't quite a Markov kernel, since it maps to more general measures. We had changed it toKleisli
, which is accurate, but the term is not in wide wide use, especially among those doing stats work.This PR tries to find a middle ground with
TransitionKernel
. It's descriptive and more intuitive, while still being accurate. The smart constructor has been changed back tokernel
, but we're no longer exporting it in order to avoid conflicts with other packages.Conditional meausres
Soss and now Tilde use
|
for conditional models. But this should really be at the level of measures. So you can not expressmeasure | constraint
in the case wheremeasure
is over NamedTuples, andconstraint
is a particular named tuple.One thing that may be surprising is that
|
in this PR does not change the dimensionality, but instead transforms the constrained dimension to a Dirac measure. I think this will be easier to later generalize to constraints that are not axis-aligned.Likelihoods
Some updates here, as well as introduction of
log_likelihood_ratio
.Pointwise Products
Slot names are changed to
prior
andlikelihood
for easier interpretation. I've added aniterate
method, allowingμ, ℓ = (p::PointwiseProductMeasure)
.Power measures
logdensity_def
is simplified to usesum
instead of an explicitfor
loop. Hopefully this will later make it easier to port to GPU methods.PowerWeighted
In for example GLMs or Bayesian bootstrap, we want to consider each observation to have some weight. This is an exponent on the likelihood factor, or equivalently a multiplicative weight on the log-likelihood. Since
^
is already taken (and much more common than this) I've added