MolecularAI / reinvent-scoring

Apache License 2.0
10 stars 14 forks source link

QED Calculations Class mimic Physchem Class to allow for transformations #7

Closed JanoschMenke closed 1 year ago

JanoschMenke commented 1 year ago

Currently you cannot apply any transformations on the QED Score. It will not throw an error if you specific this it will just silently not transform the scores. The new code is identical to the code used to calculate the PhyChem properties, and hence allows for transforming the QED scores like you would transform any other score

halx commented 1 year ago

Hi,

many thanks for the PR. I am not sure why you need that but it is correct that in REINVENT3 transforms are an intrinsic part of the scoring component, or not. This makes some sense but lacks generalizability obviously. We are currently testing a new scoring code where transforms are entirely independent from the actual predction calculation as they are merely applied to the output of that calculation (function composition basically).

That is why I hesitate to integrate this into this code base because it will soon be obsolete. If you still want to proceed with this PR, would you be able to also provide test code?

Many thanks again, Hannes.

JanoschMenke commented 1 year ago

Hi Hannes,

thanks for your response. I could see a the use of a right_step function to force a threshold on the QED score as practical to constrain the generation of "weird" molecules. Even if not considered practical it should throw an error/warning when a transformation is specified within the config.json for a component for which a transformation is/cannot be applied. However, as their is an incoming overhaul, there is no need to integrate this PR. I thought I just use bring it to the attention, and as I had the modified version I would do it in form of a PR.

Greetings, Janosch