[Cdk-devel] [ cdk-Patches-1897087 ] DeduceBondSystemTool fix

[SourceForge.net <no...@so...>]

Patches item #1897087, was opened at 2008-02-19 18:09
Message generated for change (Comment added) made by egonw
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=320024&aid=1897087&group_id=20024

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: None
Group: None
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: Nina Nikolova - Jeliazkova (vedina)
Assigned to: Nobody/Anonymous (nobody)
Summary: DeduceBondSystemTool fix

Initial Comment:

After some debugging - at least for benzene DeduceBondSystemTool class manages to assign correct bond orders in lines 567-572 (private loop() method)

                for (int j = 0; j <= MasterList.size() - 1; j++) {
                    ArrayList ringlist2 = (ArrayList) MasterList.get(j);
                    ArrayList bondlist = (ArrayList) ringlist2.get(choices[j]);
//                    logger.debug(j+"\t"+choices[j]);
                    applyBonds(mnew, bondlist);
                }

but then the  if (isStructureOK(mnew))  check fails ( line 576)


There is a strange piece of code in isStructureOK(), namely this one

            // do it multiple times to catch all the aromatic rings
            // this problem is that the aromaticity detector relies on
            // the aromaticity of the individual atoms in the ring
            // these wont be picked up until you do it multiple times
            // for example pyrene 129-00-0

            for (int i = 0; i <= ringSet.getAtomContainerCount() - 1; i++) {
                AtomContainerManipulator.percieveAtomTypesAndConfigureAtoms(molecule);
                CDKHueckelAromaticityDetector.detectAromaticity(molecule);
            }

I am not sure whether the comment applies to the old or to the new aromaticity detector.

Still, the culprit seem to be somewhere else (lines 671-674)

                    if (!ring.getFlag(CDKConstants.ISAROMATIC)) {
//                        logger.debug(counter+"\t"+"ring not aromatic"+"\t"+r.getAtomCount());
                        return false;
                    }

The check for the aromaticity flag for the rings fails. I don't think the CDKAromaticityDetector sets aromaticity flags for the rings - struggled with it few weeks ago (the old HueckelAromaticityDetector did set ring flags). A warning in the Javadoc will be helpful...

Anyway, replacing the aromaticity flag condition with one looking if all atoms in the ring are aromatic did the trick - it returns benzene with correct bond orders.

(code starting line 663)
                if (Check[i]) {
                    int aromaticAtoms = 0;
                    for (int j = 0; j <= ring.getAtomCount() - 1; j++) {
                        if (ring.getAtom(j).getHydrogenCount()<0) {
                            return false;
                        }
                        if (ring.getAtom(j).getFlag(CDKConstants.ISAROMATIC))
                                aromaticAtoms++;
                    }
                    /*
                    if (!ring.getFlag(CDKConstants.ISAROMATIC)) {
//                        logger.debug(counter+"\t"+"ring not aromatic"+"\t"+r.getAtomCount());
                        return false;
                    }
                    */
                    if (ring.getAtomCount() != aromaticAtoms) {
                        logger.debug(counter+"\t"+"ring not aromatic"+"\t"+ring.getAtomCount());
                        return false;
                    }
                   
                }

This is a quick fix, I haven't verified if the code works for more complex structures.

Best regards,
Nina



----------------------------------------------------------------------

>Comment By: Egon Willighagen (egonw)
Date: 2008-09-29 15:14

Message:
Nina, can you provide a patch for this?

----------------------------------------------------------------------

Comment By: Egon Willighagen (egonw)
Date: 2008-08-06 16:17

Message:
Logged In: YES 
user_id=25678
Originator: NO

Did this get applied yet?

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=320024&aid=1897087&group_id=20024