Bug: HalfCauchyLogPrior.sample

[DavAug]

Something is off with sampling from a ComposedLogPrior:

# Create composed log-prior
log_prior_1 = pints.UniformLogPrior(
    lower_or_boundaries=[1E-3] * 5,
    upper=[1E3] * 5)
log_prior_2 = pints.HalfCauchyLogPrior(location=0, scale=3)
log_prior = pints.ComposedLogPrior(log_prior_1, log_prior_2)

# Sample from log-prior
log_prior.sample(n=10)

Throws the following error

---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
 in 
     80 
     81 # Define initial starting points by sampling from prior
---> 82 log_prior.sample(n=10)
     83 

~/pints/_log_priors.py in sample(self, n)
    271             lo = hi
    272             hi += prior.n_parameters()
--> 273             output[:, lo:hi] = prior.sample(n)
    274         return output
    275 

ValueError: could not broadcast input array from shape (10) into shape (10,1)

[DavAug]

@MichaelClerx @chonlei @ben18785 @martinjrobins
I think it's just that for priors that are one dimensional, line 273 doesn't work because the returned array by prior.sample(n) is one dimensional. So I'll just fix that.

The problem didn't show up because the method was tested with GaussianLogPriors which already return samples of dimension 2, i.e. shape (n,1).

I guess the question would be whether we want just all sample methods of log_priors to return 2 dimensional arrays irrespective of the number of parameters (i.e. we leave the ComposedLogPrior as it is), or we adapt the ComposedLogPrior.

[chonlei]

whether we want just all sample methods of log_priors to return 2 dimensional arrays irrespective of the number of parameters

I thought that's the intention. We've been treating parameters as "parameter vectors" elsewhere -- even if it was 1 dimensional parameter, we would need it to be an array of 1 element -- e.g. in OptimisationController or MCMCController (if I remember correctly).

[MichaelClerx]

    def sample(self, n=1):
        """
        Returns ``n`` random samples from the underlying prior distribution.

        The returned value is a numpy array with shape ``(n, d)`` where ``n``
        is the requested number of samples, and ``d`` is the dimension of the
        prior.

        Note: This method is optional, in the sense that only a subset of
        inference methods require it.
        """
        raise NotImplementedError

[chonlei]

I suppose it's just a one line fix?

    def sample(self, n=1):
        """ See :meth:`LogPrior.sample()`. """

        # use inverse transform sampling
        us = np.random.uniform(0, 1, n)
        return np.array([self.icdf(u) for u in us])

to

    def sample(self, n=1):
        """ See :meth:`LogPrior.sample()`. """

        # use inverse transform sampling
        us = np.random.uniform(0, 1, n)
        return np.array([[self.icdf(u)] for u in us])

[chonlei]

Fixed by #1196