A multivariate lognormal using MvNormal internally. #455
Conversation
|
One squashed commit with code, tests, and documentation all in one. This is new functionality (a new multivariate distribution) that doesn't touch existing code. @kmsquire or @andreasnoack would you mind checking and merging? |
| @@ -207,6 +209,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) | |||
There was a problem hiding this comment.
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! and scale! below.
Can you open an issue for defining these for other distributions?
There was a problem hiding this comment.
@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.
|
LGTM. If there aren't any other comments, I'll merge in the next day or two. |
| @@ -44,6 +44,7 @@ export | |||
| ContinuousMultivariateDistribution, | |||
| ContinuousMatrixDistribution, | |||
| SufficientStats, | |||
| AbstractMvLogNormal, | |||
There was a problem hiding this comment.
I think this can wait until there'll be more subtypes.
|
@KrisDM Generally a good PR. The documentation and tests are great. I'm in doubt about the special I also know it is a convention for log-normals, but it also a bit weird that Finally, another time please remove whitespace in a separate request. It adds a lot of noise to the diff. |
|
@andreasnoack Thanks. I'll remove the whitespace, will resubmit later this afternoon (I blame Juno with its default setting of 2 :-) - I have changed it now to 4). location! and scale!. As pattern it follows sqmahal!, which I think is also defined for one distribution only (MvNormal). As for the conversions itself:
An alternative solution to exporting location! and scale! would be to provide additional constructors, and supply an optional symbol to signal how to interpret the parameters (e.g., :location as the default, :meancov, :mean, :median and :mode for the ones relying on the conversions). I would then leave the internal _location! and _scale! functions to do the actual calculations, not export the location! and scale! functions, and leave the location and scale functions exposed for use cases that need a conversion without creating a distribution. I agree it's ironic that location and scale were not defined for univariate LogNormal. Opened an issue to look into it. |
Implements the same constructors as MvNormal, as well as the interface
discussed in the Distributions documentation.
No changes to the existing code from Distributions.
The tests are very similar to the ones for MvNormal itself.
Also contains functionality to calculate the scale and location for a lognormal given
some desired statistics for the distribution.
This functionality is needed in e.g., MCMC methods, where you need to
center a distribution around e.g. the median or mode.
In particular, there are functions that allow to calculate:
(1) location and scale for a given mean and covariance
(2) location for a given scale and either mean, median or mode
(location and scale cannot be calculated analytically from e.g., mode and
covariance)
The added scale and location functions are the equivalent of static class
functions in C++ (typed on ::Type{MvLogNormal}) to ensure correct
dispatch.
I added tests to test/mvlognormal.jl for all functionality
Added documentation for the multivariate lognormal distribution.
Fixing indentation and removed AbstractMvLogNormal from exported types.
|
I cleaned up the indentation and the AbstractMvLogNormal. Ready to go in my view... |
A multivariate lognormal using MvNormal internally.
A port of my private MvLogNormal class to the Distributions package. Implemented the minimum interface described in the documentation and tested using similar tests as MvNormal.
Implements the same constructors as MvNormal.
No changes to the existing code from the Distributions package (other than includes and exports).