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

Archimedeans #73

Merged
merged 10 commits into from
Nov 20, 2023
Merged

Archimedeans #73

merged 10 commits into from
Nov 20, 2023

Conversation

Santymax98
Copy link
Contributor

Hello, I think I fixed the equations and the calculation of the tau corresponding to these copulas.

I fixed InvGaussianCopula's tau calculation
fix tau InvGaussianCopula
I fixed the tau calculation for the Gumbel-Barnett copula
I fixed the tau calculation for the Inverse Gaussian copula
Copy link

codecov bot commented Nov 17, 2023

Codecov Report

Attention: 21 lines in your changes are missing coverage. Please review.

Comparison is base (2dfce3d) 79.64% compared to head (a3ca09a) 80.40%.
Report is 3 commits behind head on main.

Files Patch % Lines
src/ArchimedeanCopulas/AMHCopula.jl 54.16% 11 Missing ⚠️
src/ArchimedeanCopulas/InvGaussianCopula.jl 74.07% 7 Missing ⚠️
src/ArchimedeanCopulas/FrankCopula.jl 85.71% 2 Missing ⚠️
src/ArchimedeanCopulas/GumbelBarnettCopula.jl 90.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #73      +/-   ##
==========================================
+ Coverage   79.64%   80.40%   +0.75%     
==========================================
  Files          27       27              
  Lines         570      643      +73     
==========================================
+ Hits          454      517      +63     
- Misses        116      126      +10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@lrnv
Copy link
Owner

lrnv commented Nov 17, 2023

Looks like the gumbelBarnett tau does not return anything ? the result line is commented

solving typing error
@lrnv
Copy link
Owner

lrnv commented Nov 17, 2023

Thanks. Could you add a few coherence tests on these two functions ? e.g. test that tau(tauinv(x)) == x for two or three well-chosen x values.

@Santymax98
Copy link
Contributor Author

Santymax98 commented Nov 17, 2023 via email

@lrnv lrnv linked an issue Nov 17, 2023 that may be closed by this pull request
@Santymax98
Copy link
Contributor Author

Hello @lrnv , I was trying to use the tests, however I am having a problem on my machine with the quadgk function telling me that it is not defined.

@lrnv
Copy link
Owner

lrnv commented Nov 17, 2023

Yes you need to add QuadGK to the dependencies of the package ! While in the package repositoryin julia, use ``] add QuadGKand it will modify theProject.toml` file, that you can then commit. I will do it for you if you want but RN i'm on the bus back home, maybe tomorow

Copy link
Owner

@lrnv lrnv left a comment

Choose a reason for hiding this comment

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

I pushed to your branch with the fix for the missing package. But I still have weird behaviors in the tests... Cheking by hand,

julia> Copulas.τ(GumbelBarnettCopula(2,0.7))
-0.2726863504658321

whihc is really weird since it is supposed to be only positive dependence for this copula ?

-> The integrated function might be wrong ?

Same thing for the InvGaussian.

@Santymax98
Copy link
Contributor Author

Santymax98 commented Nov 17, 2023 via email

Copy link
Owner

@lrnv lrnv left a comment

Choose a reason for hiding this comment

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

So I marked the test as Broken... this makes it pass, but this is not good, something must be done about those tests to pass them into "non-broken" state again, this is not acceptable.

@lrnv
Copy link
Owner

lrnv commented Nov 17, 2023

Indeed gumbelbarnett allows negative dependency, i corrected that sorry. But the tests are still broken :)

@Santymax98
Copy link
Contributor Author

Santymax98 commented Nov 17, 2023 via email

@lrnv
Copy link
Owner

lrnv commented Nov 20, 2023

Now the tests are passing, I will merge this if @Santymax98 you agree ?

@Santymax98
Copy link
Contributor Author

Santymax98 commented Nov 20, 2023 via email

@lrnv
Copy link
Owner

lrnv commented Nov 20, 2023

My pleasure ! We fixed a lot of stuff with these tests that is really good.

@lrnv lrnv merged commit 4e22ab1 into lrnv:main Nov 20, 2023
7 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.

Fix broken kendall tau on GumbelBarnettCopula and InvGaussianCopula
2 participants