Mehei/otherinferences #203
Conversation
heimengqi
force-pushed
the
mehei/otherinferences
branch
from
3a14046 to
20cf21b
Compare
…e marginal_effect_inference
…om _interval() directly
…inference when T0 equals to T1
…EconML into mehei/otherinferences
…between coef/intercept__interval() and coef/intercept__inference().conf_int() and some minor bug fix
| coef_title = 'Coefficient Results' | ||
| smry.add_table(coef_array, coef_headers, coef_stubs, coef_title) | ||
| except Exception as e: | ||
| print("Coefficient Results: ", str(e)) |
There was a problem hiding this comment.
Is there a particular exception you expect to be thrown here? Why is it okay to just continue?
There was a problem hiding this comment.
If X is None, there is not coefficient to return but we can still return intercept inference. Similarly, if cate_fit_intercept is False, we could only output coefficient inference.
There was a problem hiding this comment.
I see - in that case, it seems like it would be better to explicitly check for those conditions rather than catching any exception and proceeding.
There was a problem hiding this comment.
But there are multiple possibilities that we couldn't output intercept inference or coef inference or both of them, like user might input an unseen T1 or input T1 the same as control, should I check all of them one by one? I would probably leave it for today beta release and polish it later, I think we still have a lot of other properties we could output under this summary function.
There was a problem hiding this comment.
If you want to postpone, that's fine with me. However, I think it's usually useful to differentiate between things failing for reasons that you know about (e.g. there is no X and that's fine) as opposed to unexpected reasons (e.g. something has the wrong shape inside of coef__inference due to a bug in our logic and we throw an exception which we would want to bubble up, but we swallow the exception here and never notice).
| @@ -1,712 +1,804 @@ | |||
| # Copyright (c) Microsoft Corporation. All rights reserved. | |||
There was a problem hiding this comment.
Any idea why the GitHub diff shows the whole file as changing? I can't really review it in this format...
There was a problem hiding this comment.
Looks like maybe a problem with the line endings... See if you can keep your changes but revert to Unix-style line endings.
There was a problem hiding this comment.
when I committed, there was a conflict on this file because of the update on master. I used the web interface to solve and conflict and for some reason github identified the whole file has been changed...I compared the two file adding my changes without changing anything for the current file on master. Don't know exactly how to recover the previous version, please help here if we should go back to the previous master version to make sure everything is unchanged and add my stuff again.
There was a problem hiding this comment.
I think if you make any trivial change to this file (e.g. add or remove a blank line at the end of the file) on your own machine and commit it then all of the line endings will be fixed automatically.
add other inferences functionalities for non bootstrap case.