`ChemDataReader.read_data` incorrectly accepts entire invalid strings with atleast one valid token

[aditya0by0]

The read_data method in ChemDataReader currently parses any string that contains at least one valid token, even if the rest/entire string as whole is invalid. This leads to invalid strings being fed to the model.

        ....
        ....
        try:
            return [self._get_token_index(v[1]) for v in _tokenize(raw_data)]
        except ValueError as e:
            print(f"could not process {raw_data}")
            print(f"\t{e}")
            return None

Because of this logic, an entire invalid string with just one valid token gets parsed and is accepted and fed to the model for classification.

Eg. the string ADASDAD is parsed because there is a valid token S in it. So the entire string is represented as by [64].

64 here corresponds to the index of S token.

[print(v) for v in _tokenize("ADASDAD")]
(<TokenType.ATOM: 1>, 'S')

self._get_token_index('S') 
64

This should be avoided and the reader should return None. Hence, this logic must be improvised.

[aditya0by0]

Also, I wonder why was the logic introduced at the first place. Is there any particular purpose served or an edge case handled by this logic ?

[aditya0by0]

Also, there is another issue with this logic, even though the entire string is invalid without any valid token in it. For such strings, the code written in try block is always executed and hence an exception is never raised so the code of except block is redundant and never executed .

In case of entire invalid string an empty list is returned,

self._read_data('ADADAD')
[]

this empty list is turned into a tensor of zeros by the collator and this is fed to the model for classification.

smiles in batch = ['Cn1c(=O)c2c(ncn2C)n(C)c1=O', 'ADADAD', 'CO']

tensor of the batch:

tensor([[19, 69, 42, 41, 16, 63, 28, 17, 41, 59, 41, 16, 69, 41, 69, 59, 19, 17,
         69, 16, 19, 17, 41, 42, 63, 28],
        [ 0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
          0,  0,  0,  0,  0,  0,  0,  0],
        [19, 28,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
          0,  0,  0,  0,  0,  0,  0,  0]])

[sfluegel05]

I guess the edge case where we might want this behaviour is a SMILES string that is mostly correct (except for one spelling mistake or one rare SMILES feature that is not supported by the SMILES parser).
However, in general it would be better to only accept fully valid SMILES strings. The easiest way to ensure this would be to convert the SMILES to an rdkit mol object and then reproduce the SMILES (which we are already doing for canonicalisation).