Adding iSIM as a trackable value during training

[khuddzu]

Sorry it took me so long to get to this.
I have made the changes needed in the REINVENT code to track the iSIM average similarity across each training step, and add this value to the tensorboard output.

Below is a list of the changes I made:

1. In learning.py I have imported iSIM, computed binary RDKIT fingerprints from the smilies array, and computed isim from the generated fingerprints. I then added isim as a variable to the report_data dictionary.
2. In data.py I have added the isim variable to RLReportData.
3. In tensorboard.py I have added iSIM:Average Similarity to the report as a scalar per each step.

I want to mention that I started with the absolute barebones steps for this implementation. I want to make sure this is what you all are looking for.

Below are other changes I can make:

1. I have coded this by importing iSIM functions from the installable package. This now makes the iSIM package a dependency for REINVENT. If this is not what you all want I can either hard code the iSIM functions I used into the script, or add some variable so that iSIM is only imported if the user sets this to True.
2. For the fingerprint generation I am currently computing binary fingerprints from RDKIT. We support different fingerprint types in this function, I can also make this optional for the user
3. With iSIM, we can compute different types of average pairwise similarities. In this current implementation I have used Tanimoto. I can also make it optional for the user to choose the other similarity types we support.

Please let me know if this was not what you were looking for, or if you are interested in any of the above additions. Also, let me know if there was a place in the REINVENT code that I missed adding the iSIM variable. With the test code I ran, this successfully adds the iSIM value per step in the tensorboard event file.

Best,
Kate Huddleston

[halx]

Many thanks for this Kate! The code so far looks very good.

The iSIM functionality should be optional i.e. a switch in the TOML file should control whether the user wants to see this or not. As for fingerprint/distance choice, I would leave that open for the moment.

One problem though is that iSIM does not seem to have a proper installation procedure. The specific issue is that a pip install does not copy the necessary files into the installation location. The assumption is being made that the package will be installed in editable mode. That is not really an option for the end user. Eventually it should be possible to install via pip install git+ssh://[email protected]/mqcomplab/iSIM.git because that we can than add to the REINVENT installation setup.

[khuddzu]

I have made iSIM installable via:
pip install git+ssh://[email protected]/mqcomplab/iSIM.git

I am trying to fix the issue with making isim tracking an optional variable in the TOML file. I am tracking back from learning.py and data.py and trying to find how the config file variables are accessed and treated. I am running into a bit of an issue locating this.
Any advice you could give would be greatly appreciated.

Thanks for your patience.

[halx]

Many thanks.

Best would be to add a parameter tb_isim to runmodes/RL/validation.py in class SectionParameters. You would the need to pass this in from run_staged_learning.py (via config.tb_isim) via the call to model_learning (almost at the end of the file).

[khuddzu]

I now have implemented isim tracking as optional. I had to make changes in a few more places than you mentioned.
For the toml file, the config variable tb_isim must be under [parameters].
Let me know if you have any questions or concerns about the changes I have made.

[halx]

Ok, but we would also need to update the requirements file and the pyproject.toml file

[halx]

Please, set up your IDE or editor to not add spurious whitespace. This distracts from actual code review. Thanks.

[halx]

This is not needed as global parameter because it is only relevant in the RL parameter section.