Temporal shenanigans #286

jmafoster1 · 2024-07-25T14:51:30Z

Added in temporal causal estimation with treatment sequences.

github-actions · 2024-07-25T14:52:32Z

🦙 MegaLinter status: ⚠️ WARNING

Descriptor	Linter	Files	Fixed	Errors	Elapsed time
⚠️ PYTHON	black	30		1	1.03s
✅ PYTHON	pylint	30		0	4.71s

See detailed report in MegaLinter reports

MegaLinter is graciously provided by

codecov · 2024-07-26T13:02:48Z

Codecov Report

Attention: Patch coverage is 93.79310% with 9 lines in your changes missing coverage. Please review.

Project coverage is 95.55%. Comparing base (5af464f) to head (e6284c5).
Report is 2 commits behind head on main.

Files	Patch %	Lines
causal_testing/testing/causal_test_adequacy.py	64.00%	9 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #286      +/-   ##
==========================================
- Coverage   95.62%   95.55%   -0.07%     
==========================================
  Files          22       23       +1     
  Lines        1530     1666     +136     
==========================================
+ Hits         1463     1592     +129     
- Misses         67       74       +7

Files	Coverage Δ
causal_testing/json_front/json_class.py	`97.98% <100.00%> (ø)`
causal_testing/specification/capabilities.py	`100.00% <100.00%> (ø)`
causal_testing/testing/causal_test_case.py	`100.00% <ø> (+4.54%)`	⬆️
causal_testing/testing/causal_test_outcome.py	`96.92% <100.00%> (+0.09%)`	⬆️
causal_testing/testing/estimators.py	`92.28% <100.00%> (+3.01%)`	⬆️
causal_testing/testing/causal_test_adequacy.py	`87.17% <64.00%> (-11.13%)`	⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b6ce637...e6284c5. Read the comment docs.

…alTestingFramework into temporal-shenanigans

f-allian

@jmafoster1 Looks good so far. Seems like a really seamless integration. I'm currently not on my laptop so I'll do further tests later this evening / tomorrow.

Also, it might be worth requesting a review from @christopher-wild if he has some time.

f-allian · 2024-07-29T14:06:28Z

causal_testing/specification/capabilities.py

+    def total_time(self):
+        """
+        Calculate the total duration of the treatment strategy.
+        """
+        return (len(self.capabilities) + 1) * self.timesteps_per_intervention


@jmafoster1 Out of curiosity, why is the total time defined as (n+1)*dt instead of n*dt? Do you assume the treatment strategy starts at t=dt instead of t=0?

@f-allian, yes exactly. This is slightly geared towards the water case study, where there is a grace period of dt before the first intervention takes place, and another period of dt after the last intervention before we can observe the final outcome. A slightly more general solution might be to have an extra grace_period parameter and then have the total time be len(capabilities) * timesteps_per_intervention + grace_period, but I'll keep it as it is for now.

f-allian · 2024-07-29T14:15:30Z

tests/specification_tests/test_capabilities.py

+
+    def test_repr(self):
+        cap = Capability("v", 1, 0, 1)
+        self.assertEqual(str(cap), "(v, 1, 0-1)")


@jmafoster1 Should this be "(v, 1, 0, 1)" instead of "(v, 1, 0-1)"?

I thought the 0-1 notation for stringing capabilities would be easier to read, then you know that the last element represents the duration of the capability. Otherwise, I'd be constantly forgetting what the last three elements represented!

f-allian · 2024-07-29T14:32:27Z

tests/testing_tests/test_estimators.py

@@ -77,7 +79,7 @@ class TestLogisticRegressionEstimator(unittest.TestCase):

    @classmethod
    def setUpClass(cls) -> None:
-        cls.scarf_df = pd.read_csv("tests/data/scarf_data.csv")
+        cls.scarf_df = pd.read_csv("tests/resources/data/scarf_data.csv")


@jmafoster1 Why did this change to test/resources/data? Do we have other subfolders in tests/resources?

There was a folder called tests/data and one called tests/resources/data, both containing test data, so I thought I'd condense everything into one folder.

jmafoster1 · 2024-07-29T14:46:50Z

@jmafoster1 Looks good so far. Seems like a really seamless integration. I'm currently not on my laptop so I'll do further tests later this evening / tomorrow.

Also, it might be worth requesting a review from @christopher-wild if he has some time.

Thanks @f-allian. @christopher-wild, I've requested a review, but please don't feel obliged if you are too busy

causal_testing/specification/capabilities.py

christopher-wild · 2024-07-30T14:21:46Z

causal_testing/testing/causal_test_adequacy.py

+            try:
+                results.append(self.test_case.execute_test(estimator, None))
+            except LinAlgError:
+                continue


Would logging any warnings here be helpful? Is it okay if it fails to append results?

I've added in the warnings, but it's very OK to continue. What's happening here is that we're resampling the data and recalculating the estimate. If you don't have a lot of data, some of those samples may fail to converge, or fail in other ways to produce an estimate. This indicates that you don't have enough data, so I've now added "number of successes" as a third dimension to causal test adequacy.

causal_testing/testing/estimators.py

tests/specification_tests/test_capabilities.py

jmafoster1 · 2024-08-02T12:39:35Z

If everyone's happy with this, please could one of you approve it so I can get it merged in?

christopher-wild · 2024-08-02T13:48:52Z

If everyone's happy with this, please could one of you approve it so I can get it merged in?

Ah sorry missed this! You can click re-request review next to ours name in the top right to send a notification if we do take too long

jmafoster1 · 2024-08-02T13:50:37Z

Thanks @christopher-wild. Doing that feels very passive aggressive, though! 🙃

christopher-wild · 2024-08-02T13:53:16Z

Haha I get what you mean but it's what it's there for 😁!

jmafoster1 added 5 commits July 23, 2024 16:32

Temporal estimation in

4951029

Fixed pyproject.toml

b321170

Fixed estimation bug

0d76e92

Fixed causal test adequacy unit tests

e2aef75

pytest and linting

392387c

jmafoster1 and others added 2 commits July 26, 2024 09:07

Merge branch 'main' into temporal-shenanigans

cff1776

fix: tests in pandas version > 2

bffe827

f-allian and others added 6 commits July 26, 2024 14:03

fix: linting

fbfe76a

Some more linting

6ceef1a

Merge branch 'temporal-shenanigans' of github.com:CITCOM-project/Caus…

c20899d

…alTestingFramework into temporal-shenanigans

Merge branch 'main' into temporal-shenanigans

e985641

pytest

41ea949

Merge branch 'temporal-shenanigans' of github.com:CITCOM-project/Caus…

c1c64c2

…alTestingFramework into temporal-shenanigans

jmafoster1 requested a review from f-allian July 29, 2024 09:20

jmafoster1 marked this pull request as ready for review July 29, 2024 09:20

Test capabilities

a619069

f-allian reviewed Jul 29, 2024

View reviewed changes

jmafoster1 requested a review from christopher-wild July 29, 2024 14:46

christopher-wild reviewed Jul 30, 2024

View reviewed changes

jmafoster1 and others added 3 commits July 31, 2024 08:10

Minor corrections

4abe735

pylint

2b76d3c

Merge branch 'main' into temporal-shenanigans

e6284c5

christopher-wild self-requested a review August 2, 2024 13:48

christopher-wild approved these changes Aug 2, 2024

View reviewed changes

jmafoster1 merged commit 2b7042d into main Aug 2, 2024
14 of 16 checks passed

jmafoster1 deleted the temporal-shenanigans branch August 2, 2024 13:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Temporal shenanigans #286

Temporal shenanigans #286

jmafoster1 commented Jul 25, 2024

github-actions bot commented Jul 25, 2024 •

edited

Loading

codecov bot commented Jul 26, 2024 •

edited

Loading

f-allian left a comment

f-allian Jul 29, 2024

jmafoster1 Jul 29, 2024

f-allian Jul 29, 2024

jmafoster1 Jul 29, 2024

f-allian Jul 29, 2024

jmafoster1 Jul 29, 2024

jmafoster1 commented Jul 29, 2024

christopher-wild Jul 30, 2024

jmafoster1 Jul 31, 2024

jmafoster1 commented Aug 2, 2024

christopher-wild commented Aug 2, 2024

jmafoster1 commented Aug 2, 2024

christopher-wild commented Aug 2, 2024

Temporal shenanigans #286

Temporal shenanigans #286

Conversation

jmafoster1 commented Jul 25, 2024

github-actions bot commented Jul 25, 2024 • edited Loading

🦙 MegaLinter status: ⚠️ WARNING

codecov bot commented Jul 26, 2024 • edited Loading

Codecov Report

f-allian left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmafoster1 commented Jul 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmafoster1 commented Aug 2, 2024

christopher-wild commented Aug 2, 2024

jmafoster1 commented Aug 2, 2024

christopher-wild commented Aug 2, 2024

github-actions bot commented Jul 25, 2024 •

edited

Loading

codecov bot commented Jul 26, 2024 •

edited

Loading