inclusion of covariates on penalized SC

[dhvalden]

This is a question rather than an issue. I noticed that the penalised SC does not take into consideration the covariates means for the inner optimization. Did I miss something or is it by design?

    def fit(self, dataprep: Dataprep, lambda_: float) -> None:
        """Fit the model/calculate the weights.

        Parameters
        ----------
        dataprep : Dataprep
            :class:`Dataprep` object containing data to model.
        lambda_ : float
            Ridge parameter to use.
        """
        self.dataprep = dataprep
        self.lambda_ = lambda_

        X0_df, X1_df = dataprep.make_outcome_mats()
        X0, X1 = X0_df.to_numpy(), X1_df.to_numpy()

        W, _ = self.w_optimize(X0=X0, X1=X1, lambda_=lambda_)
        self.W = W

[sdfordham]

Yes this looks like an error, it looks like I put the X0 and X1 matrices there instead of the Z0 and Z1 matrices (in the notation of the 2003 Abadie et al paper). The notation in the penalised paper uses X for what is Z in the 2003 paper (which this codebase uses). I will check this in more detail and make a PR to fix this.

[dhvalden]

Yes this looks like an error, it looks like I put the X0 and X1 matrices there instead of the Z0 and Z1 matrices (in the notation of the 2003 Abadie et al paper). The notation in the penalised paper uses X for what is Z in the 2003 paper (which this codebase uses). I will check this in more detail and make a PR to fix this.

I don't think it is an error per se. The min function is excluding the covariates, or more formally, assuming equal weight for all of them (is actually excluding the covariates). The results are good tho. And iirc, Abadie does not explicitly mention the covariates in that paper, so that's why I asked if it was by design. Thanks a lot anyway 😊

[sdfordham]

I have finally looked at this, and I see now that my implementation of this version of SCM is fundamentally flawed. It is not implementing the paper and needs to be re-written, I'm going to start this process now.

[sdfordham]

@v1.3.0 has a correct implementation now of the paper. Thank you for raising this issue 👍.