From: SourceForge.net <no...@so...> - 2008-02-19 17:09:42
|
Patches item #1897087, was opened at 2008-02-19 19:09 Message generated for change (Tracker Item Submitted) made by Item Submitter 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 ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=320024&aid=1897087&group_id=20024 |