Fix Shapley method #158
Fix Shapley method #158
Conversation
Vaibhavdixit02
merged commit bedb96e
into
compathelper/new_version/2023-11-14-23-08-53-265-03909794485
| # conditional sampling in independent random vector is just subset sampling. | ||
| Array(rand(Copulas.subsetdims(distribution, idx), n_sample)') # this might need to be transposed. | ||
| samples = zeros(eltype(x_cond), length(idx), n_sample) |
There was a problem hiding this comment.
Preallocation is a bit safer, otherwise the eltype of the samples can be a bit surprising (see below).
| if n_dep == 1 | ||
| dist_cond = Normal(cond_mean[1, 1], cond_var[1, 1]) | ||
| sample_norm = rand(dist_cond, n_sample) | ||
| dist_cond = Normal(cond_mean[1, 1], sqrt(cond_var[1, 1])) |
There was a problem hiding this comment.
The second argument of Normal is the standard deviation.
| # The copula returns samples of `Float64`s | ||
| θ = convert(AbstractArray{Float32}, θ) |
There was a problem hiding this comment.
I think this is an upstream problem - the copula (and hence also input_distribution) lost the type information and generate samples of type Float64.
There was a problem hiding this comment.
Yep Copulas.jl was intended to be type-agnostic at the beginning, but sometimes it gets it wrong, as Distributions.jl… see, e.g., there. And since I did not write tests from the beginning for this particular feature, I forgot about it.
Opened an issue, will look at it when I have a bit of time (not right now, unfortunately) For the moment you have to keep this convertion as far as i can tell, sorry about that.
|
Sorry for the delay in fixing it I know you guys were waiting on it but I got busy and this wasn't as trivial as it seemed. Great stuff as always @devmotion, thanks! |
This PR seems to fix the docs issues in #134 locally.