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

Add support for creating correlated variables from a covariance or correlation matrix #137

Merged
merged 3 commits into from Apr 5, 2023

Conversation

LukasACH
Copy link
Contributor

This PR adds four new functions:

Measurements.covariance_matrix(n::AbstractVector{<:Measurement})

to calculate the covariance matrix from a vector of measurements,

Measurements.correlation_matrix(n::AbstractVector{<:Measurement})

to calculate the correlation matrix, and

Measurements.correlated_values(
    nominal_values::AbstractVector{<:Real},
    covariance_matrix::AbstractMatrix{<:Real}
)

and

Measurements.correlated_values(
    nominal_values::AbstractVector{<:Real},
    standard_deviations::AbstractVector{<:Real},
    correlation_matrix::AbstractMatrix{<:Real}
)

to create correlated variables from a covariance and correlation matrix respectively.

The algorithms are taken from the python package uncertatinties. If that is not okay, please tell me.

If there are issues with code styling, or if something is missing, I am happy to fix or add it.

@LukasACH
Copy link
Contributor Author

Do you want to keep the backwards compatibility to version 1.0? If yes, I will have to find some compromises regarding readability/speed with the code.

@giordano
Copy link
Member

I haven't looked into the details of the implementation, but this is awesome!

I'm happy to drop support for older versions of Julia, what would be the first one needed as things are now?

src/utils.jl Outdated Show resolved Hide resolved
@giordano
Copy link
Member

I'd appreciate documenting this in the Usage and Examples sections of the manual

@LukasACH
Copy link
Contributor Author

I will add the corresponding usage documentation and a few examples in the coming days as soon as I find some more time.

@codecov-commenter
Copy link

codecov-commenter commented Mar 23, 2023

Codecov Report

Merging #137 (d87a7b4) into master (d1ac981) will increase coverage by 0.18%.
The diff coverage is 100.00%.

❗ Current head d87a7b4 differs from pull request most recent head 45db0d4. Consider uploading reports for the commit 45db0d4 to get more accurate results

@@            Coverage Diff             @@
##           master     #137      +/-   ##
==========================================
+ Coverage   95.45%   95.63%   +0.18%     
==========================================
  Files          12       12              
  Lines         726      756      +30     
==========================================
+ Hits          693      723      +30     
  Misses         33       33              
Impacted Files Coverage Δ
src/Measurements.jl 86.66% <ø> (ø)
src/utils.jl 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@LukasACH
Copy link
Contributor Author

I have added documentation for both, could you check if it fits your expectation?

Also, are you happy with the function names? There are existing functions in Statistics named cov and cor for calculating the covariance and correlation matrices. Would it be better to extend these?

@LukasACH LukasACH requested a review from giordano March 27, 2023 15:40
docs/src/examples.md Outdated Show resolved Hide resolved
src/utils.jl Outdated Show resolved Hide resolved
src/utils.jl Outdated Show resolved Hide resolved
src/utils.jl Outdated Show resolved Hide resolved
@giordano
Copy link
Member

BTW, this PR solves the only point in the TODO list

TODO
----
* Allow defining quantities related by a correlation matrix and correctly
propagate uncertainty in this case
which can now be removed 🙂

@LukasACH
Copy link
Contributor Author

LukasACH commented Mar 31, 2023

Thanks for your comments, I will fix them.

What is your opinion on extending the functions in Statistics named cov and cor for calculating the covariance and correlation matrices instead of creating our own?
There are other packages like Distributions.jl which extend these functions.
If we extend Statistics, should we export cov and cor or let the user import Statistics themselves?

@LukasACH LukasACH force-pushed the correlation-covariance branch 2 times, most recently from cbf6a6b to b319b9f Compare March 31, 2023 11:16
@giordano
Copy link
Member

giordano commented Apr 5, 2023

Thanks a lot for this nice contribution, much appreciated and awaited!

@giordano giordano merged commit f1d6e89 into JuliaPhysics:master Apr 5, 2023
11 checks passed
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.

None yet

3 participants