choderalab / pinot

Probabilistic Inference for NOvel Therapeutics
MIT License
15 stars 2 forks source link

[WIP] biophysical_with_ic50 #113

Open yuanqing-wang opened 3 years ago

lgtm-com[bot] commented 3 years ago

This pull request introduces 2 alerts when merging 51878e1a2228b0c406addc1ac2f673b8f34aad9d into c8cfda1169c8349d0659cb7c3fe03eadf18e9d63 - view on LGTM.com

new alerts:

karalets commented 3 years ago

So let me add some comments here.

First, @yuanqing-wang it would be good to get into the habit of leaving descriptions about your PR, even when obvious.

Second, I see you are trying to 'polish' my left behind version of the biophysical regressor, which I appreciate. But imho we should rethink a lot of our designs anyway for the models, as the other BiophysicalGP regressor 'runs' now but all of this has lost some modularity.

I will look more into the PR to see the details.

@jchodera : the units are my bad, @yuanqing-wang has just edited a piece of code I submitted in a rush without doc. We discussed the units in our own chat for pinot, they are in Molar for the parameter concentration to be passed.

Since we have not gotten results with this that are satisfactory yet I would consider this still preliminary, I am also playing more with the other version of this code to see what we can get. give us a couple of days.

lgtm-com[bot] commented 3 years ago

This pull request introduces 3 alerts when merging fffca5bae2579a12df42ef108353a7d936f57fdd into c8cfda1169c8349d0659cb7c3fe03eadf18e9d63 - view on LGTM.com

new alerts: