Corrected cross-fitting loop

judithabk6 / med_bench

BSD 3-Clause "New" or "Revised" License

8 stars 3 forks source link

Corrected cross-fitting loop #42

Closed sami6mz closed 7 months ago

sami6mz commented 10 months ago

Previous cross-fitting loop was using an array of numpy array, and we suspected it to slow down the computation time since multiply_robust_efficient is faster than med_dml. This commit optimizes the loop.

In the end it appears the loop wasn't slowing down anything (med_dml is slower because it calls forest instances more often than multiply_robust). Also the new loop seems to give slightly more accurate estimation.

judithabk6 commented 7 months ago

ok. So to recap

the first commit corrects the way the cross-fitting is done (just rewriting, it yields the same results - should be explicit with a test, obviously)
the second commit is just documentation

As for #43 only the code in get_estimation is tested, so the package is not properly equipped to check specifically that this PR does not break the code.

Should we merge this as it is, and implement a more specific test suite? @bthirion @houssamzenati