Skip to content
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

Accept reinterpreted arrays for weighted covariance calculation #882

Open
tscode opened this issue Jul 13, 2023 · 1 comment
Open

Accept reinterpreted arrays for weighted covariance calculation #882

tscode opened this issue Jul 13, 2023 · 1 comment

Comments

@tscode
Copy link

tscode commented Jul 13, 2023

I like to keep observed variables in structs and use reinterpret to efficiently convert a vector of variables to a matrix. However, this clashes with cov when weights are given. Consider this example (I am on julia 1.9.0):

using StatsBase

struct Observation
  a :: Float64
  b :: Float64
end

obs = [Observation(1., 2.), Observation(2., 4.)]
weights = [0.5, 0.5]

data = reinterpret(reshape, Float64, obs) # get a 'reinterpreted' Float64 matrix
cov(data, dims = 2, corrected = false) # works as expected
cov(data, Weights(weights), 2) # fails

data = unsafe_wrap(Array, pointer(data), size(data))
cov(data, Weights(weights), 2) # works as expected

The issue is that reinterpret(reshape, Float64, obs) does not yield a subtype of DenseMatrix, even though it is a dense matrix.
I am not sure if this issue should be resolved by StatsBase or can be tackled upstream.

@ararslan
Copy link
Member

I am not sure if this issue should be resolved by StatsBase or can be tackled upstream.

The relevant method is defined by StatsBase, so this is the right place. Thanks for the report!

The issue is that reinterpret(reshape, Float64, obs) does not yield a subtype of DenseMatrix

Based on a brief look at the implementation, I don't see a reason why the methods need to be restricted to DenseMatrix; it seems to me that any AbstractMatrix could work, including your example. We could try simply loosening the argument's type restriction for the relevant method definitions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants