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 KS-statistic example #61

Merged
merged 19 commits into from
Dec 17, 2023
Merged

Add KS-statistic example #61

merged 19 commits into from
Dec 17, 2023

Conversation

Datseris
Copy link
Member

@Datseris Datseris commented Dec 4, 2023

Closes #4, #29.

This PR contributes a new example that does what the above issues requested. It also allows the possibility of using nothing as indicators, in which case the indicator analysis is completely skipped and the change metric is estimated directly from input timeseries.

Still todo:

  • Test that nothing works with surrogate analysis
  • add some tests.

@Datseris Datseris requested a review from JanJereczek December 4, 2023 15:54
@Datseris
Copy link
Member Author

@JanJereczek I need your help. I do not understand why my newest test fails. See my @testset "nothing indicator" begin in the full_analysis.jl test file. When I compute the significant flags, every single timepoint gets significance. I don't understand why. Only one timepoint, that where c > 1 should be significant. at all other timepoints the change metric value should not be significant.

This is the plot of the change metric:

image

I don't understand what's going on.

@Datseris
Copy link
Member Author

tests fail here because our p-value counting code is wrong. It become wrong when we changed it to the current code, which admittedly is incredibly complex and overengineered. The performance of pvalues is miniscule so we should simplify the code so that bugs are transparent and easy to find.

In any case the PR here is fine.

I also added some plotting code that I will finish in a different PR. I'll go ahead and merge this in and we can work on the individual issues later.

@Datseris Datseris merged commit 64bb7af into main Dec 17, 2023
0 of 2 checks passed
@Datseris Datseris deleted the kolm_smironv branch December 17, 2023 19:07
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.

Kolmogorov-Smirnov test as an indicator of a transition
1 participant