-
Notifications
You must be signed in to change notification settings - Fork 422
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
A multivariate lognormal using MvNormal internally. #455
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,7 +9,7 @@ using StatsBase | |
using Compat | ||
|
||
import Base.Random | ||
import Base: size, eltype, length, full, convert, show, getindex, scale, rand, rand! | ||
import Base: size, eltype, length, full, convert, show, getindex, scale, scale!, rand, rand! | ||
import Base: sum, mean, median, maximum, minimum, quantile, std, var, cov, cor | ||
import Base: +, -, .+, .- | ||
import Base.Math.@horner | ||
|
@@ -99,6 +99,7 @@ export | |
MixtureModel, | ||
Multinomial, | ||
MultivariateNormal, | ||
MvLogNormal, | ||
MvNormal, | ||
MvNormalCanon, | ||
MvNormalKnownCov, | ||
|
@@ -207,6 +208,7 @@ export | |
sqmahal, # squared Mahalanobis distance to Gaussian center | ||
sqmahal!, # inplace evaluation of sqmahal | ||
location, # get the location parameter | ||
location!, # provide storage for the location parameter (used in multivariate distribution mvlognormal) | ||
mean, # mean of distribution | ||
meandir, # mean direction (of a spherical distribution) | ||
meanform, # convert a normal distribution from canonical form to mean form | ||
|
@@ -221,6 +223,7 @@ export | |
ncomponents, # the number of components in a mixture model | ||
ntrials, # the number of trials being performed in the experiment | ||
params, # get the tuple of parameters | ||
params!, # provide storage space to calculate the tuple of parameters for a multivariate distribution like mvlognormal | ||
pdf, # probability density function (ContinuousDistribution) | ||
pmf, # probability mass function (DiscreteDistribution) | ||
probs, # Get the vector of probabilities | ||
|
@@ -230,6 +233,7 @@ export | |
rate, # get the rate parameter | ||
sampler, # create a Sampler object for efficient samples | ||
scale, # get the scale parameter | ||
scale!, # provide storage for the scale parameter (used in multivariate distribution mvlognormal) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note that It's probably okay here (especially since @StefanKarpinski, I often want to cite one of your talks or an issue when saying this, but I can't find a good one--any suggestions? Assuming, of course, that I'm not misrepresenting the idea. (@KrisDM, this probably has no direct impact on this PR, just things to keep in mind.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @kmsquire @StefanKarpinski True, but it's an occasion where Julia's out-of-class function definitions show a problem of - excuse the pun - scalability. "scale" has a different meaning for matrices as it has for distributions. In a C++ class for the lognormal distirbution, we wouldn't think twice about using scale as a function name (even if the standard library would contain an entirely unrelated function scale) because the class context would define the semantics of the word, I have to say that one thing that surprised me about Julia is that if a method like "scale" is defined in base, and I want to also define it in my module, I have to import and then export it again to avoid warnings (which happened the last time I tried this in 0.3 - haven't tried this anymore in 0.4 so apologies if already changed). A more scalable behaviour, in my view, would be to allow separate definition in the two namespaces, and if I import the two definitions into the same workspace, let the mulitple-dispatch figure out which one to call. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems like a defensibly similar usage of "scale" to me, so I would just extend Base's versions of these. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But if you don't feel so, don't listen to me – I'm not an expert on this domain. |
||
shape, # get the shape parameter | ||
skewness, # skewness of the distribution | ||
span, # the span of the support, e.g. maximum(d) - minimum(d) | ||
|
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'm a little hesitant to export this without support for more distributions, but it does follow the pattern of many other functions defined here. Same for
params!
andscale!
below.Can you open an issue for defining these for other distributions?
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.
@kmsquire I opened an issue (#459). The in-place variants would only be relevant to multivariate distributions, and the other multivariates that have been implemented to date may not have a location or scale, but I'll double-check it.
I also realized that location and scale should be defined for some univariate distributions that use that terminology, yet some are missing. I opened #460 for that.