From: SourceForge.net <no...@so...> - 2010-08-30 16:10:15
|
Patches item #3040273, was opened at 2010-08-05 16:35 Message generated for change (Comment added) made by raj_76 You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=320024&aid=3040273&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: cdk-1.2.x Group: Needs Review Status: Open Resolution: None Priority: 5 Private: No Submitted By: Saravanaraj AN (raj_76) Assigned to: Nobody/Anonymous (nobody) Summary: Error in SmilesGenerator Initial Comment: The SmilesGenerator creates a wrong SMILES string for this molecule BrC1C(Br)C(Br)C(Br)C(Br)C1Br The SMILES created is C1C(C(C(C(C1Br)Br)Br)Br)BrBr but the correct SMILES string is C1(C(C(C(C(C1Br)Br)Br)Br)Br)Br. The SMILES generator is missing a parenthesis around the first branching and creates a molecue with the two bromines connected to each other. I traced through the code and found what I think is the problem. In the ParseChain method, around line 1535 the code checks to see if the parent has a ring break with the atoms of the branch and if the connected bond count for the parent is less than 4. I think that this logic will not work if the parent atom is the very first atom. The code should check to see if the parent atom is the very first atom and if it is then the connected bonds count should be checked for less than 3 instead of 4. Hope this makes sense. -Raj ---------------------------------------------------------------------- >Comment By: Saravanaraj AN (raj_76) Date: 2010-08-30 12:10 Message: I attached a patch with a unit test to reproduce the bug. ---------------------------------------------------------------------- Comment By: Rajarshi Guha (rajarshi) Date: 2010-08-29 15:56 Message: I applied the unti test ot cdk-1.2.x but skipped the fix. But the test passes, so I can't reproduce the problem. Also, when the unit test patch is updated, please ensure that the unit test goes into org.openscience.cdk.smiles.SmilesGeneratorTest ---------------------------------------------------------------------- Comment By: Egon Willighagen (egonw) Date: 2010-08-29 11:32 Message: The patch looks sane to me, but in order for this fix to go into CDK 1.2.x we need a unit test. I'll attach one, but it does not fail without the fix. Put differently, I do not know how to reproduce the problem. Raj, please check my patch with the unit test and tell me what I am doing wrong in this test, and/or write a unit test that fails without your bug fix. ---------------------------------------------------------------------- Comment By: Saravanaraj AN (raj_76) Date: 2010-08-18 13:31 Message: Yes, the patch is the file attached to this report. ---------------------------------------------------------------------- Comment By: Egon Willighagen (egonw) Date: 2010-08-18 13:01 Message: I suggest to aim this fix at CDK 1.2.x... ---------------------------------------------------------------------- Comment By: Egon Willighagen (egonw) Date: 2010-08-18 12:59 Message: Dear Raj, this is the patch attached to this report, correct? Thanx! ---------------------------------------------------------------------- Comment By: Saravanaraj AN (raj_76) Date: 2010-08-18 12:45 Message: I have a patch for this issue that fixes it. I will mail it to the cdkpatches_at_gmail com address. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=320024&aid=3040273&group_id=20024 |