Skip to content

Conversation

@andreasnoack
Copy link
Member

Fixes #8080

scale!(unscaled_covzm(x, vardim), inv(size(x,vardim) - Int(corrected)))
function covzm(x::AbstractMatrix, vardim::Int=1; corrected::Bool=true)
C = unscaled_covzm(x, vardim)
T = typeof(first(C) / 1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can unscaled_covzm ever return heterogeneously-typed arrays?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can. Are you worried about first not being representative of the elements in C?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should usually be a subtype of the element type, but other elements might be wider for abstractly typed arrays, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that the element type has to be fairly concrete before we can instantiate it but I've just realized that in this case here, it should be possible to use promote_type(typeof(first(C) / 1), eltype(x)). I'll update.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eltype(x) or eltype(C)? will unscaled_covzm ever result in a different eltype?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably rare. I can only think of Bool entries and there is wouldn't change T but I think you are right that I should use C and not x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maths Mathematical functions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants