From: Egon W. <ego...@gm...> - 2008-01-20 19:21:21
|
On Jan 20, 2008 1:07 PM, Miguel Rojas Cherto <m....@la...> wrote: > On Fri, 2008-01-18 at 22:37 +0100, Egon Willighagen wrote: > > do you remember what was wrong with the code of BreakingBondReaction, > > I remember that we did't solvent the problem. We found where was the > problem. It was with the atomTypeMatcher I think. And you opened a bug > for this: Bugs item #1850861. This is covered by getting a CDKAtomTypeMatcher in this way: CDKAtomTypeMatcher.getInstance( NoNotificationChemObjectBuilder.getInstance(), CDKAtomTypeMatcher.REQUIRE_EXPLICIT_HYDROGENS ); The latter makes sure that it cannot compensate missing neighbors by accounting them as implicit hydrogens. > > where we first had 1 reactions, then fixed something but had too many > > reactions then (6 I think)? Did you commit what we worked on then? Or > > did we do that on my machine? Because the current version in SVN just > > gives one... > > We worked with your Desktop because my Laptop was broken (still). But I > can not remember if we worked with others reaction than > BreakingBondReaction. OK, I'm about to commit a fix to the CDK atom types, which, I think, had incorrect properties for: C.minus.sp2 which should be the atom type for [H]-[C-]=O, so 2 neighbors (was 3) and sp2 hybridization, and C.plus.sp2 which should be the atom type for [H]-[C+]=O, so 2 neigbors (was 3) and one pi-bond (was 0). It causes no failing unit tests... which you may interpret as both negative and positive :) Anyway, it fixes the BreakingBondReaction... Miguel, what we should do, is for all tests for these reactions, write atom type perception tests for the products that should be formed. In this way, we split IReactionProcess problems from atom typing problems... Egon -- ---- http://chem-bla-ics.blogspot.com/ |
From: Egon W. <ego...@gm...> - 2008-01-21 12:14:39
|
On Jan 21, 2008 8:53 AM, Miguel Rojas Cherto <m....@la...> wrote: > > what we should do, is for all tests for these reactions, write atom > > type perception tests for the products that should be formed. In this > > way, we split IReactionProcess problems from atom typing problems... > > Ok, Perfect. I actually found two problems in the SMILES parser and generator... filed bug reports... > I will write Tests today for atom type perception in each Reaction So, please create IMolecule instances in tests, instead of using SMILES... Please make individual tests for each reactant and product... for any of them, including radical species... Any radical molecule will currently fail, but that's only because I have not written up any radical atom type yet... Next major task is, I think, the StructureResonanceGenerator... that seems the most broken right now... (BTW, I also added tests for parameter settings...) Egon -- ---- http://chem-bla-ics.blogspot.com/ |
From: Egon W. <ego...@gm...> - 2008-01-21 20:40:40
|
On Jan 21, 2008 1:14 PM, Egon Willighagen <ego...@gm...> wrote: > Next major task is, I think, the StructureResonanceGenerator... that > seems the most broken right now... Ah... more radicals. BTW, the progress for the branch is starting to look good: egonw@egonw-laptop:~/var/Projects/SourceForge/cdk$ diff -u trunk.results branch.results | grep "^-T" -Testcase: testResonancePositiveCharge_1(org.openscience.cdk.test.qsar.descriptors.bond.ResonancePositiveChargeDescriptorTest) -Testcase: testResonancePositiveCharge_2(org.openscience.cdk.test.qsar.descriptors.bond.ResonancePositiveChargeDescriptorTest) -Testcase: testBB_AutomaticSearchCentreActiveFormaldehyde(org.openscience.cdk.test.reaction.type.BreakingBondReactionTest) -Testcase: testResonanceFluoroethylene(org.openscience.cdk.test.tools.StructureResonanceGeneratorTest) egonw@egonw-laptop:~/var/Projects/SourceForge/cdk$ diff -u trunk.results branch.results | grep "^+T" +Testcase: testAllylRadical(org.openscience.cdk.test.tools.StructureResonanceGeneratorTest) +Testcase: testKetoEnol(org.openscience.cdk.test.tools.StructureResonanceGeneratorTest) +Testcase: testBenzene(org.openscience.cdk.test.tools.StructureResonanceGeneratorTest) +Testcase: testResonanceStructure(org.openscience.cdk.test.smiles.SmilesParserTest) These four tests are not yet available in trunk/. First is a missing radical; the second a real bug; the third seems to be a bug: it cannot find the two kekule structures of benzene (removed symmetry by adding to side groups); the fourth is a bug in the SmilesParser (filed on SF). Egon -- ---- http://chem-bla-ics.blogspot.com/ |