[Cdk-devel] one more possible correction for the UniversalIsomorphismTester class

[Irilenia N. <no...@eb...>]

I think that when subgraph matching is performed, it is not generally a 
good idea to match pseudoatoms. However, the class as it is written now 
does not check that two atoms are not pseudo atoms and since pseudo atoms 
get the symbol R, they are always matched. I think this is not what one 
would generally expect. 

So I suggest that a test should be put in place so that pseudo atoms are 
not matched by the nodeConstructor() funtion. The change is probably 
something very simple like:


//bond type conditions
...blah blah
//atom type conditions
//add pseudoatom conditions here
&&
                (
                   (!(bondsA1[i].getAtomAt(0) instanceof PseudoAtom))
                   && (!(bondsA1[i].getAtomAt(1) instanceof PseudoAtom))
                   && (!(bondsA2[j].getAtomAt(0) instanceof PseudoAtom))
                   && (!(bondsA2[j].getAtomAt(1) instanceof PseudoAtom))
                )
&&
//continue with atom type conditions
( // a1 = a2 && b1 = b2 ...etc

The only problem is again the case single atoms. These cannot be dealt
within the nodeContstructor (as we are talking here about atoms and 
not bonds) and
must be dealt in the search() method (admittedly this is becoming more and 
more of a problem, and there should come a time when we find a better 
solution for single atom case matching rather than keeping on adding code 
to the search() method! - I wonder what Thierry's company is doing in 
this case? Do they have a more elegant solution?).
Anyway, as things are now, we need the following changes to apply the
pseudoatom filter:
In the "Test for single atom cases" part, we need to change:

FROM:

if(g1.getAtomAt(i).getSymbol().equals(atomSymbol)
    arrayList.add(new RMap(i,0));

TO 

if(g1.getAtomAt(i).getSymbol().equals(atomSymbol) && (!(g1.getAtomAt(i) 
instanceof PseudoAtom)))
    arrayList.add(new RMap(i,0));

and
FROM:
if(g2.getAtomAt(i).getSymbol().equals(atomSymbol)
    arrayList.add(new RMap(0,i));
TO:

if(g2.getAtomAt(i).getSymbol().equals(atomSymbol) && (!(g2.getAtomAt(i) 
instanceof PseudoAtom)))
    arrayList.add(new RMap(0,i));

(well,if you've applied my previous suggestion, the code may look a little 
bit different as you need to add the arrayList to the rMapsList too but 
basically the solution is rather simple).

Unless someone disagrees strongly, I think it only makes sense not to 
match R groups during subgraph matching.

Thanks to Egon for applying the changes,

I.

++++++++++++++++++++++++++++++++++++++++
Irilenia (Irene) Nobeli
EMBL - European Bioinformatics Institute
Wellcome Trust Genome Campus
Hinxton
Cambridge CB10 1SD
tel: +44 (0) 1223 492550
fax: +44 (0) 1223 494468
e-mail: no...@eb...