Skip to content

Feature/kucherenko indices#276

Open
sitoryu wants to merge 28 commits into
masterfrom
feature/kucherenko_indices
Open

Feature/kucherenko indices#276
sitoryu wants to merge 28 commits into
masterfrom
feature/kucherenko_indices

Conversation

@sitoryu

@sitoryu sitoryu commented Oct 16, 2025

Copy link
Copy Markdown
Contributor

Implementation of 2 methods to calculate Kucherenko Indices and ability to create conditional samples from a gaussian copula

New functions:

Standard Kucherenko Indices using conditional sampling
Sample based Kucherenko Indices using binning
create conditional samples from a gaussian copula

@FriesischScott
FriesischScott self-requested a review October 16, 2025 13:00
@FriesischScott FriesischScott added enhancement Improvement of existing code feature New feature or feature request and removed enhancement Improvement of existing code labels Oct 16, 2025

@FriesischScott FriesischScott left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please move both the conditional sampling and the corresponding tests into joint distribution files.

Overall we need to be able to pass both dependent and independent inputs to the kucherenkoindices function at the same time so we can for example compute the sensitivity for the cantilever example.

Comment thread src/sensitivity/kucherenkoindices.jl Outdated
end


function _compute_first_order_kucherenko(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this can be moved inside the higher up function.

Comment thread src/sensitivity/kucherenkoindices.jl Outdated
return S_i
end

function _compute_total_effect_kucherenko(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same for this function.

Comment thread src/sensitivity/kucherenkoindices.jl Outdated
joint_dist::JointDistribution,
var_names::Vector{Symbol}
)
input_var_names = [marginal.name for marginal in joint_dist.m]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is just names(joint_dist).

Comment thread src/inputs/jointdistribution.jl Outdated
return DataFrame(permutedims(rand(jd.d, n)), jd.m)
end

function sample_conditional_copula(joint::JointDistribution{<:GaussianCopula,<:RandomVariable}, var_values::Vector{Tuple{Symbol,Float64}}, N::Int)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it would be good to have a function that takes a JointDistribution and a DataFrame with less columns than the joint distribution requires and returns the data frame with the remaining columns samples conditionally based on the existing samples.

Comment thread src/sensitivity/kucherenkoindices.jl Outdated
- `min_bin_sample::Int=25`: Minimum samples per bin for 1 dimension for conditioning; Recommended amount is at least 25 samples per bin with bin amount around 100
- `min_bin_sample_multi_dims::Int=25`: Minimum samples per bin-dimension in multiple dimensions for conditioning; Recommended amount is about 10-25 samples per bin
"""
function kucherenkoindices_bin(X::Matrix, Y::Vector; min_bin_sample=nothing, min_bin_sample_multi_dims::Int=25)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should dispatch on a DataFrame with all the data and an output Symbol for which to compute the sensitivity indices. This way all the variable names are available as well.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

And keep the name kucherenkoindices.

Comment thread src/sensitivity/kucherenkoindices.jl Outdated
- `min_bin_sample::Int=25`: Minimum samples per bin for 1 dimension for conditioning; Recommended amount is at least 25 samples per bin but bin amount not higher than 100
- `min_bin_sample_multi_dims::Int=25`: Minimum samples per bin-dimension in multiple dimensions for conditioning; Recommended amount is about 10-25 samples per bin
"""
function kucherenkoindices_bin(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

By changing the first method to a DataFrame you can get rid of all of these and it will be up to the user if they want to use the conditional sampling or the binning.

Comment thread test/inputs/copulas/gaussian.jl Outdated
@testset "sample_conditional_copula" begin
marginals = [RandomVariable(Normal(0,1), :x1), RandomVariable(Normal(0,1), :x2), RandomVariable(Normal(0,1), :x3)]
joint_dist = JointDistribution(GaussianCopula([1 0.7071 0.5; 0.7071 1 0.3; 0.5 0.3 1]), marginals)
unconditional = sample(joint_dist, 500000)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we need a few more tests. We have to make sure that the conditional samples have the correct strength of dependence but also that they are resulting from a Gaussian copula.

@FriesischScott

Copy link
Copy Markdown
Member

@sitoryu With the Copulas merged in #286 you can now update your code to use conditional sampling provided by Copulas.j

Unfortunately, there are a few conflicts to fix first.

@codecov

codecov Bot commented Mar 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.03139% with 20 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/sensitivity/kucherenkoindices.jl 91.44% 13 Missing ⚠️
src/inputs/jointdistribution.jl 90.14% 7 Missing ⚠️

📢 Thoughts on this report? Let us know!

@sitoryu

sitoryu commented Mar 13, 2026

Copy link
Copy Markdown
Contributor Author

@FriesischScott
I updated the kucherenko indices to work with the Copulas.jl package and as to previous comments.

Regarding the new conditional sample functions - I implemented two versions:

function sample(jd::JointDistribution{<:Copulas.Copula,<:RandomVariable}, conditions::AbstractDict{<:Symbol,<:Real}, n::Integer=1)

and

function sample(jd::JointDistribution{<:Copulas.Copula,<:RandomVariable}, existing_samples::DataFrame)

Not sure if we want to keep both, they are also currently lacking tests, which I'll add after deciding which ones to keep

@FriesischScott

Copy link
Copy Markdown
Member

function sample(jd::JointDistribution{<:Copulas.Copula,<:RandomVariable}, existing_samples::DataFrame)

Not sure what exactly the AbstractDict does in the first one but I definitely prefer this one. Does this work with RandomVariable{ProbabilityBox}?

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

Labels

feature New feature or feature request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants