ronikobrosly / causal-curve

A python package with tools to perform causal inference using observational data when the treatment of interest is continuous.
MIT License
271 stars 18 forks source link

cannot import name 'GPS' #33

Closed v6l4188 closed 3 years ago

v6l4188 commented 3 years ago

Hello~ Thank you for your package, but now I have a problem using it. from causal_curve import GPS when I use this, the error occurs as: ImportError: cannot import name 'GPS'

I tried to modify it to be: from causal_curve import gps_core but when I establish the module by: math_gps = gps_core(gps_family='normal', lower_grid_constraint = 0.0, upper_grid_constraint = 0.99, n_splines=10, verbose=False) There is another error, and it says that module is not callable.

So I modified it to be: math_gps = gps_core.GPS_Core(gps_family='normal', lower_grid_constraint = 0.0, upper_grid_constraint = 0.99, n_splines=10, verbose=False) And then I fit the model, but when I calculate the CDRC: results_dict['math_CDRC'] = math_gps.calculate_CDRC() The error occurs as: AttributeError: 'GPS_Core' object has no attribute '_cdrc_predictions_continuous'

Therefore, how can I solve this problem? Thank you for your help :D

ronikobrosly commented 3 years ago

Hi @v6l4188 thanks for your interest! A few weeks ago I did a major update to the package and the API was changed. What version of the package are you using? If you check the latest documentation the import statement for version 1.x is now “from causal_curve import GPS_Regressor” or “from causal_curve import GPS_Classifer”. You can see lots of examples in the documentation: https://causal-curve.readthedocs.io/en/latest/index.html

But maybe you were looking at a part of the documentation that I forgot to update? Can I ask where you saw that you needed to import just “GPS”?

v6l4188 commented 3 years ago

Hi @v6l4188 thanks for your interest! A few weeks ago I did a major update to the package and the API was changed. What version of the package are you using? If you check the latest documentation the import statement for version 1.x is now “from causal_curve import GPS_Regressor” or “from causal_curve import GPS_Classifer”. You can see lots of examples in the documentation: https://causal-curve.readthedocs.io/en/latest/index.html

But maybe you were looking at a part of the documentation that I forgot to update? Can I ask where you saw that you needed to import just “GPS”?

Thank you for your reply! The version of the package is the newest one, and I modified it as you said, it works~ Exactly as you said, I'm looking at a part of the documentation, and it is still importing "GPS". They are the examples here: https://causal-curve.readthedocs.io/en/latest/causal_curve.html#module-causal_curve.gps_core https://github.com/ronikobrosly/causal-curve/blob/master/examples/NHANES_BLL_example.ipynb It seems that I managed to miss all correct ones :)

And there is one more question now. I tried to reproduce the result of your end-to-end example in https://github.com/ronikobrosly/causal-curve/blob/master/examples/NHANES_BLL_example.ipynb, I didn't change any code in it (except changing "GPS" to be "GPS_Regressor"), but the figure I got is different from the figure in the example (see the attachment). Is there anything wrong? (The histograms I got were the same as the example, so I think the problem doesn't lie in data) try_causal

ronikobrosly commented 3 years ago

@v6l4188 this is helpful because you’re pointing out the bits I forgot to update! I hastily put a note at the top of the example notebook about the API changing but I really should update the notebook so it uses the new API. I’ll take a look at the final causal curves at the end too. There were slight tweaks to the models that could cause that, but let me dig a bit more.

ronikobrosly commented 3 years ago

@v6l4188 i should be able to fix these in the next few days. I’ll update you here.

ronikobrosly commented 3 years ago

Ok @v6l4188 , I just updated the package to version 1.0.2 which includes updates to the end-to-end jupyter notebook so that it incorporates the latest API. I fixed some of the docstrings you were looking at so they also use the latest API. Also, you discovered a bug in my major package update! The classes weren't actually using custom inputs. So in the jupyter notebook, where the code is: math_gps = GPS_Regressor(gps_family='normal', lower_grid_constraint = 0.0, upper_grid_constraint = 0.99, n_splines = 5, verbose=False), it wasn't actually using any of these params and was instead using the defaults.

Could I get you to install the latest version and try running the notebook for yourself to verify that it worked.

v6l4188 commented 3 years ago

Ok @v6l4188 , I just updated the package to version 1.0.2 which includes updates to the end-to-end jupyter notebook so that it incorporates the latest API. I fixed some of the docstrings you were looking at so they also use the latest API. Also, you discovered a bug in my major package update! The classes weren't actually using custom inputs. So in the jupyter notebook, where the code is: math_gps = GPS_Regressor(gps_family='normal', lower_grid_constraint = 0.0, upper_grid_constraint = 0.99, n_splines = 5, verbose=False), it wasn't actually using any of these params and was instead using the defaults.

Could I get you to install the latest version and try running the notebook for yourself to verify that it worked.

Thank you for your updated version! I installed the last version, and the causal curves that I get are correct now. Unfortunately, now at the next step "Exploring how BLLs mediate the relationship between income and cognitive outcomes" in the example, I also get a result different from the example you offered. At the step of: med = Mediation( bootstrap_draws=500, bootstrap_replicates=100, spline_order=3, n_splines=5, verbose=True, ) med.fit( T=final_df["PIR"], M=final_df["BLL"], y=final_df["Math"], ) med_results = med.calculate_mediation(ci = 0.95)

The output is: Mean indirect effect proportion: 0.2018 (0 - 0.3888) It is different from the example you offered. and at last, the figure of the mediation effect of bloodlead on PIR, I got this figure, which is also different from that of your example. May you please carefully check it again? And it will be appreciated if you can try to look for all potential bugs in your code :D thank you very much. 下载

ronikobrosly commented 3 years ago

@v6l4188 I believe that’s because I didn’t set the random seed in that particular example. There is a stochastic mechanism within the mediation tool so without setting the seed results will vary a bit run to run. If you set the seed when you call the Mediation tool it should stay the same run to run. As long as your result is close-ish to mine I think you got it to work.

ronikobrosly commented 3 years ago

@v6l4188 , I just posted another update to the package. It fixes an issue with the random_seed parameter and I now set the random seed in the example notebook.

Could you install version 1.0.3 and verify you get exactly the same results when using the Mediation tool.

v6l4188 commented 3 years ago

@v6l4188 , I just posted another update to the package. It fixes an issue with the random_seed parameter and I now set the random seed in the example notebook.

Could you install version 1.0.3 and verify you get exactly the same results when using the Mediation tool.

Yes, I have got exactly the same results using version 1.0.3! Thank you very much, now I have no questions.