judithabk6 / med_bench

BSD 3-Clause "New" or "Revised" License
8 stars 3 forks source link

need to correct IPW code for y and t input format #14

Open judithabk6 opened 1 year ago

judithabk6 commented 1 year ago

IPW implementation behaves badly if t and y dimension is (n, 1), and ok if t and y dimension is (n, 0), please fix

sami6mz commented 1 year ago

I just realized IPW is not the only one to fail. g_computation, multiply_robust, simulation_based return ValueError if .ravel() is not applied to t and y. I don't know about others estimators yet, but I will soon implement a test in response to #15.

sami6mz commented 1 year ago

I justed tested .ravel() robustness for all estimators G_estimator badly behaves too (in addition to IPW, g_computation, multiply_robust, and simulation_based)

sami6mz commented 1 year ago

As for multiply_robust, (n,1) format makes the function to fail because some tr variable is not initialised : l688 - l689

if len(t.shape) == 1:
        tr = t.reshape(-1, 1)

Instead of :

if len(t.shape) == 1:
        tr = t.reshape(-1, 1)
else:
        tr = np.copy(t)
        t = t.ravel()

Just like m.

Besides, why copying t or m is needed?

sami6mz commented 1 year ago

Besides, why copying t or m is needed?

Okay I realized get_interactions didn't support (n,) inputs so that's why the copying t and m was needed. Issue was made in #34, and is to be solved in #35.

sami6mz commented 1 year ago

Just a reminder, as for now :

judithabk6 commented 1 year ago

need to modify the test accordingly when solved, in particular https://github.com/judithabk6/med_bench/blob/13c97dadd3209f1fdb6e8b845225a88d89f0e6e0/tests/estimation/test_get_estimation.py#L185 and https://github.com/judithabk6/med_bench/blob/13c97dadd3209f1fdb6e8b845225a88d89f0e6e0/tests/estimation/test_get_estimation.py#L195 (remove the .ravel())