sissa-data-science / DADApy

Distance-based Analysis of DAta-manifolds in python
https://dadapy.readthedocs.io/
Apache License 2.0
99 stars 16 forks source link

Causality dev #105

Closed vdeltatto closed 7 months ago

vdeltatto commented 7 months ago

Proposed changes Added methods to speed up repeated grid searches in the causality test, when a loop is carried out over different values of tau, and to handle the presence of confounders / indirect links.

-New methods (return_ranks_present_for_all_weights and return_inf_imb_causality_input_rank) in the MetricComparison class to avoid recomputing the ranks in space A=(alpha*X(0),Y(0)) when only Y(tau) is changed -Added the possibility of computing distances with PBC in the causality functions (arguments period_cause, period_effect and period_conditioning); -Inserted a new test in ./tests/test_information_imbalance.py; -Modified file ./tests/rosslers_xtoy_trajectory.npy to speed up tests; -Modified methods (return_inf_imb_causality and _return_inf_imb_causality_target_rank) and new method (return_inf_imb_causality_conditioning) to cope with third system Z, with a search grid over a 2D parameter space; -Added functions _return_period_present, _return_period_mixed and _compute_2d_grid to simplify code in ./dadapy/_utils/metric_comparisons.py to simplify additions in MetricComparison class.

codecov-commenter commented 7 months ago

Codecov Report

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

Comparison is base (564df28) 80.22% compared to head (4402453) 79.99%.

:exclamation: Current head 4402453 differs from pull request most recent head fe9c8a5. Consider uploading reports for the commit fe9c8a5 to get more accurate results

Files Patch % Lines
dadapy/metric_comparisons.py 64.10% 14 Missing :warning:
dadapy/_utils/metric_comparisons.py 73.91% 6 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #105 +/- ## ========================================== - Coverage 80.22% 79.99% -0.24% ========================================== Files 15 15 Lines 2432 2489 +57 ========================================== + Hits 1951 1991 +40 - Misses 481 498 +17 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

AldoGl commented 7 months ago

Hi @vdeltatto, this all looks great to me! The only thing you might consider writing an additional test in order not to bring the total coverage below 80%. You should be able to check the Codecov report given above, see what lines are not covered, and design an efficient test in this respect

vdeltatto commented 7 months ago

Hi @vdeltatto, this all looks great to me! The only thing you might consider writing an additional test in order not to bring the total coverage below 80%. You should be able to check the Codecov report given above, see what lines are not covered, and design an efficient test in this respect

Hi @AldoGl, thanks! I added a few tests to bring the coverage above 80%, but looking at the Codecov report I see that some lines which are actually covered by the last tests are shown as not covered. For example, the full function return_inf_imb_causality_conditioning seems not covered but there is actually a test for it (test_return_inf_imb_causality_conditioning). Do you have any clue why this happens?

AldoGl commented 7 months ago

Hi @vdeltatto, thanks for the great work and for the new tests. I have seen the problem you mentioned with Codecov. I honestly do not know why we get it, but I would not care too much. At this point I would just merge the PR!

vdeltatto commented 7 months ago

@AldoGl Perfect, thanks!!