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

Off-diagonal values may no longer be nonzero if the diagonal is zero for a covariance matrix #20

Merged
merged 13 commits into from
Jun 25, 2024

Conversation

billdenney
Copy link
Contributor

@mattfidler
Copy link
Member

I'm uncomfortable with the breaking change as it is written (while I agree with the need). lotri could be used in contexts other than covariances (though unlikely).

@billdenney
Copy link
Contributor Author

Should I add an argument like isCov=TRUE that enables this error or when set to FALSE, it would not trigger the error?

@mattfidler
Copy link
Member

mattfidler commented Jun 23, 2024

Yes, that would be the theory. I used cov instead and added a few test conditions.

Also this needs to be tested in nested/occasion cases

@mattfidler
Copy link
Member

I also saw some edge cases that are not handled correctly by adding this flag...

@mattfidler
Copy link
Member

It could be why the environment grows in certain circumstances...

@mattfidler
Copy link
Member

I have mostly finished. I will upload when I get computer internet access again.

The only oustanding piece for me is why is this an error instead of a warning that corrects the matrix?

The reason I ask is since this is now only applied for covariance matrices, lotri should also check for positive definite matrices (which the code will). Since getting a positive definite matrix is not always intuitive, the code can optionally find the nearest positive definite matrix by providing some sort of correction funtion.

This would make the zero correction strategy proposed here inconsistent with the non positive definite correction.

I may be missing why this should be an error instead of a correction and warning.

@billdenney
Copy link
Contributor Author

I would think that typos would be the most common reason it would have a nonzero covariance with a zero variance. In that case, I would rather see an error than have an auto correction that may not be the correction I wanted.

@mattfidler
Copy link
Member

@billdenney

Let me know if this looks good to you now.

@billdenney
Copy link
Contributor Author

Looks good to me. Thanks!

@mattfidler mattfidler merged commit ce48471 into main Jun 25, 2024
8 checks passed
@mattfidler mattfidler deleted the rxode2-481 branch June 25, 2024 11:18
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

Successfully merging this pull request may close these issues.

rxode2() allows correlation with zero diagonal values in etas
2 participants