search

uta-smile / RetroXpert

MIT License
63 stars 17 forks source link

one question about the canonicalize_products.py #10

Closed teslacool closed 3 years ago

teslacool commented 3 years ago

@chaoyan1037 If I understand correctly, the SMILES in train.csv before processed by the canonicalize_product.py is canonicalized by RDKit through smiles=Chem.MolToSmiles(mol, canonical=True). So, what is the purpose to permute the atommapnumber in this canonicalize_product.py and is there any reference work?

chaoyan1037 commented 3 years ago

@teslacool It is a good catch. We found there was an information leak within the USPTO dataset itself. The order of the atoms within the product SMILES may indicate the reaction atoms. To be more specific, we found that for most USPTO products, the first atoms of the product SMILES are usually reaction atoms. You may look into the reaction atoms yourself. So we use canonicalize_products.py to rearrange the atom order to be the same as the canonical atom order, hoping to remove the potential information leak.

teslacool commented 3 years ago

Ok. thanks for your answer.

chaoyan1037 commented 3 years ago

We have an important update of our method. Please refer to the readme for more details.

YanjingLiLi commented 1 year ago

For this leakage, does it only appear in your developed model, or it's actually a general problem for all the models using this dataset?

najwalb commented 1 year ago

@YanjingLiLi I believe it's a general problem, it was mentioned by other methods later as well: https://openreview.net/pdf?id=SnONpXZ_uQ_

Methods using atom-mapping information shld be particularly careful with this.