[AsadGudzevic94]

Description

_x_data_subset of DoubleMLDIDBinary and DoubleMLDIDCSBinary is now a property that reads the current x_cols, instead of an array stored at initialization. The short model of sensitivity_benchmark() (also the one of DoubleMLDIDMulti, which refits its group-time models) is then fitted without the benchmarking covariates.

New test doubleml/did/tests/test_did_sensitivity_benchmark.py: the delta_theta of the benchmark equals the difference to a model fitted without the covariates, for DoubleMLDIDMulti and the binary models, with panel data and repeated cross-sections. It fails on main (all zeros).

Reference to Issues or PRs

Fixes #430

PR Checklist

• The title of the pull request summarizes the changes made.
• The PR contains a detailed description of all changes and additions.
• References to related issues or PRs are added.
• The code passes all (unit) tests.
• Enhancements or new feature are equipped with unit tests.
• The changes adhere to the PEP8 standards.

[JanTeichertKluge]

Hi @AsadGudzevic94, thank you very much for the fix!
The fix appears to miss models fitted with n_jobs_models > 1.

>>> import copy
... import numpy as np
... import doubleml as dml
... from doubleml.did.datasets import make_did_CS2021
... from sklearn.linear_model import LinearRegression, LogisticRegression
... 
... np.random.seed(42)
... df = make_did_CS2021(n_obs=500, dgp_type=1, time_type="float")
... data = dml.DoubleMLPanelData(
...     df, y_col="y", d_cols="d", id_col="id", t_col="t",
...     x_cols=["Z1", "Z2", "Z3", "Z4"],
... )
... base = dml.did.DoubleMLDIDMulti(
...     data, LinearRegression(), LogisticRegression(), gt_combinations="standard"
... )
... 
... serial = copy.deepcopy(base).fit(n_jobs_models=1)
... parallel = copy.deepcopy(base).fit(n_jobs_models=2)
... 
... print("Long-model coefficients equal:", np.allclose(serial.coef, parallel.coef))
... print("Serial delta_theta:", serial.sensitivity_benchmark(["Z1"])["delta_theta"].iloc[0])
... print("Parallel delta_theta:", parallel.sensitivity_benchmark(["Z1"])["delta_theta"].iloc[0])

On the PR head, this prints:

Long-model coefficients equal: True
Serial delta_theta: -3.162367713523086
Parallel delta_theta: 0.0`

On main the behavior is different and returns:

Long-model coefficients equal: True
Serial delta_theta: 0.0
Parallel delta_theta: 0.0

it looks like parallel fitting returns group-time models with separate copies of the data object. Updating the parent models x_cols for benchmarking therefore does NOT update those copies.

Could you please check/fix this and add a parallel-fitting test?

Thank you!
Best
Jan

[AsadGudzevic94]

Thanks @JanTeichertKluge, good catch! With n_jobs_models > 1 the models come back from the workers with their own copies of the data, so changing x_cols on the parent never reached them. fit() now points them back to the parent's data, same as a serial fit.

I added n_jobs_models=2 to the test, and your example now gives the same delta_theta for serial and parallel (-3.162). One thing to note: the new parallel cases passed on their own, but their workers got killed when I ran the whole DID suite with pytest -n 4 on Windows. Plain pytest -m ci shouldn't run into that.