Conversation
causalpy/skl_meta_learners.py
Outdated
| def summary(self): | ||
| raise(NotImplementedError()) | ||
|
|
||
| "Prints summary. Conent is undecided yet." |
There was a problem hiding this comment.
| "Prints summary. Conent is undecided yet." | |
| "Prints summary. Content is undecided yet." |
|
Hi @matekadlicsko. Sorry for the slow reply, some crazy things happened in my life! Thanks for the work on this! Once things are more progressed, would you be good to:
|
|
Hi @drbenvincent , I hope everything's fine. Sure, I'll add a few tests as well as soon as I'm ready with implementing all the basic functionalities I envisioned. |
|
I think this has to do with the lack of doc-strings. Try commenting https://github.com/pymc-labs/CausalPy/blob/main/.pre-commit-config.yaml#L36-L42 and then run it again. You can also run |
pyproject.toml
Outdated
| # For an analysis of this field vs pip's requirements files see: | ||
| # https://packaging.python.org/discussions/install-requires-vs-requirements/ | ||
| dependencies = [ | ||
| "aesera", |
There was a problem hiding this comment.
aesara shouldn't be required, we changed the backend to pytensor in pymc 5.0.
There was a problem hiding this comment.
It was suspicious to me as well, but couldn't figure out why the tests failed with ERROR - ModuleNotFoundError: No module named 'aesara'. I think pymc_bart must be causing this error. Could it be because I did not specify the version requirement for it?
There was a problem hiding this comment.
Is it not pymc-bart with a dash and not an underscore?
|
I don't think I'll be able to work on this a lot this week unfortunately. I'll share where it lacks and what my plans are for (hopefully) the next week.
|
# Split data to treated and untreated subsets
X_t, y_t = X[treated == 1], y[treated == 1]
X_u, y_u = X[treated == 0], y[treated == 0]
# Estimate response function
_fit(treated_model, X_t, y_t, coords)
_fit(untreated_model, X_u, y_u, coords)
pred_u_t = az.extract(
untreated_model.predict(X_t), group="posterior_predictive", var_names="mu"
).mean(axis=1)
pred_t_u = az.extract(
treated_model.predict(X_u), group="posterior_predictive", var_names="mu"
).mean(axis=1)
tau_t = y_t - pred_u_t
tau_u = y_u - pred_t_u
# Estimate CATE separately on treated and untreated subsets
_fit(treated_cate_estimator, X_t, tau_t, coords)
_fit(untreated_cate_estimator, X_u, tau_u, coords)the posterior predictive |
I think this is a fantastic job and I also had planned to explore this. Take your time. I hope to re-visit the latest changes soon. Also, I think it should not be perfect. We can break down the ambitious proposal into some milestones. |
|
@matekadlicsko let me know whenever you need another review (no rush) |
|
@juanitorduz unfortunately I have a bit less time nowadays, but I'm progressing slowly. I'm pretty much done with skl_meta_learners.py for now. I would be very interested in your opinion about summary.py. It is definitely not in its final form, but I would love to have some feedback. The PyMC version is not yet ready for review unfortunately. |
|
Sure! No rush! Take your time. I'll be off for a couple of weeks and come back for a review 🤜🤛 |
…alPy into meta-learners merged
|
@matekadlicsko It looks like the work in EDIT: In fact, just a suggestion, but this PR might be trying to do a lot in one go. Feel free to break out the HTML summary output into a separate PR if you want. Up to you though :) |
Implemented basic functionalities for frequentist S, T and X learners. Documentations and further features are next. After that, I'm implementing PyMC compatible counterpart.