MartinSpindler / hdm

Other
11 stars 8 forks source link

ToDos: Implementation of AutoDML #10

Open PhilippBach opened 1 year ago

PhilippBach commented 1 year ago

Some todos for the package implementation

klosins commented 1 year ago

@PhilippBach thank you! I checked off first difference task bc that is already implemented. Question, for ATE do we still want to use older dictionary style de-bias or the new "M" style that we did for APD?

PhilippBach commented 1 year ago

HI @klosins ,

thanks! I'm sorry, I think I have forgotten to delete the old implementations of the ATE and APD , which I now did in https://github.com/MartinSpindler/hdm/commit/a31d23a2ff82f8062b1840b1f55117485cd49b03

So: We can use the same rlassoAutoDML function for any data backend, either the one for ATE or the one for APD

One thing I had in mind would be to have a data backend wrapper that is based on a dictionary as provided by the user. We could use this not only to offer more flexibility (then we cannot guarantee corectness, e.g. of derivatives), but also for unit tests

klosins commented 1 year ago

in commit d5c0291 I added the ability to add weight vector

klosins commented 1 year ago

In commit #f298824 I added the interactions between covariates themselves

PhilippBach commented 12 months ago

Hi @klosins ,

one thing that is not yet done is passing the unit tests for the case with weights, so I commented out https://github.com/MartinSpindler/hdm/blob/d123e757a938baaf815657797069e7fde2f91d30/tests/testthat/test_ATEAutoDML.R#L52 and https://github.com/MartinSpindler/hdm/blob/d123e757a938baaf815657797069e7fde2f91d30/tests/testthat/test_APDAutoDML.R#L41 . I think it could be that I didn't use proper weights ( I just sampled randomly from 0,1, here ) . Do you have an idea how to fix this?

I think this is the line of code

https://github.com/MartinSpindler/hdm/blob/440786e715b8d25bcfca361a6acc065a89a9a300/R/AutoDebiased_backends.R#L211

We can also chat about it

Update: I think this was resolved already and is not an open point anymore

klosins commented 11 months ago

@PhilippBach sorry for the delay - just seeing this now - Ill take a look at it, and how about we chat sometime next week ? Might be easier to talk

PhilippBach commented 10 months ago

Related issue #12

klosins commented 10 months ago

Fixed "Dobule Check data.table warning on #reference/copy" code - should be all good now

PhilippBach commented 10 months ago

Thanks! I'll have a look