Skip to content

Conversation

sitoryu
Copy link
Contributor

@sitoryu sitoryu commented Oct 16, 2025

New pull req from inside the repo:
Original text:
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
Copy link
Contributor

@FriesischScott FriesischScott left a comment

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.

end


function _compute_first_order_kucherenko(
Copy link
Contributor

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.

return S_i
end

function _compute_total_effect_kucherenko(
Copy link
Contributor

Choose a reason for hiding this comment

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

Same for this function.

joint_dist::JointDistribution,
var_names::Vector{Symbol}
)
input_var_names = [marginal.name for marginal in joint_dist.m]
Copy link
Contributor

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).

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
Contributor

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.

- `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
Contributor

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
Contributor

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.

- `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
Contributor

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.

@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
Contributor

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.

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