Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

X-Learner: Use the same sample splits in all base models. #84

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

kklein
Copy link
Collaborator

@kklein kklein commented Aug 15, 2024

TODOs:

  • Make sure that row indices not present in cv are actually not used for training when passing cv to cross_validate.
  • Assess whether the fold-specific nuisance prediction should move to CrossFitEstimator.
  • Devise a plan as to whether synchronize_cross_fitting should be allowed to be False for the X-Learner.

Observations

import numpy as np
from sklearn.base import BaseEstimator
from sklearn.model_selection import cross_validate


class Memorizer(BaseEstimator):
    def fit(self, X, y):
        self._y = y
        print(len(y))
        return self

    def score(self, X, y):
        return 0


n_samples = 100
n_folds = 4
# We define cvs such that when combining the training and test set of every 'split', we have a strict subset of 
# the dataset (X, y). 
cvs = [
    (np.array([fold_index]), np.array(fold_index + 50)) for fold_index in range(n_folds)
]
estimator = Memorizer()

X = np.random.normal(size=(n_samples, 2))
y = np.random.normal(size=n_samples)
cross_validate(
    estimator,
    X,
    y,
    cv=cvs,
)

yields the following output:

1
1
1
1

Checklist

  • Added a CHANGELOG.rst entry

@kklein kklein linked an issue Aug 15, 2024 that may be closed by this pull request
@kklein
Copy link
Collaborator Author

kklein commented Aug 15, 2024

FYI @MatthiasLoefflerQC I created a first draft of how the same splits could be used for all base learners, including treatment models. As of now the estimates are still clearly awry, e.g. an RMSE of ~13 compared to ~0.05. This happens for both in-sample and out-of-sample estimation. I currently have no real ideas on what's going wrong; will try to make some progress still

if synchronize_cross_fitting:
cv_split_indices = self._split(
index_matrix(X, self._treatment_variants_indices[treatment_variant])
treatment_indices = np.where(
Copy link
Collaborator Author

@kklein kklein Aug 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an opaque way of turning an array [True, True, False, False, True] into an array [0, 1, 4]. Not sure if there's a neater way of doing that.

Copy link
Contributor

@MatthiasLoefflerQC MatthiasLoefflerQC Aug 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[index for index, value in enumerate(vector) if value] would work too, I guess, and is more verbose, but I like the np.where :)

@kklein kklein changed the title Use the same sample splits in all base models. X-Learner: Use the same sample splits in all base models. Aug 15, 2024
@kklein
Copy link
Collaborator Author

kklein commented Aug 15, 2024

As of now the estimates are still clearly awry, e.g. an RMSE of ~13 compared to ~0.05.

The base models all seem to be doing fine wrt their individual targets at hand. Yet, when I compare pairs of treatment effect model estimates at prediction time, it become blatantly apparent that something is going wrong:

np.mean(tau_hat_control - tau_hat_treatment)
>>> 27.051119307766754
np.mean(tau_hat_control)
>>> 14.104902455634836
np.mean(tau_hat_treatment)
>>> -12.946216852131919

Update: These discrepancies have been substantially reduced by bbfff15. The RMSEs on true cates are still massive when compared to status quo.

Co-authored-by: Matthias Loeffler <[email protected]>
model_kind=CONTROL_EFFECT_MODEL,
model_ord=treatment_variant - 1,
is_oos=False,
)
)[control_indices]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need is_oos=False below (and likewise for tau_hat_treatment)? Might be worth a try.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Leakage in X-Learner in-sample prediction
2 participants