From: Christoph S. <ste...@eb...> - 2008-09-10 08:18:09
|
Egon, I never finished the regeneration of fingerprints successfully (just lack of time) :-) So Stefan finalized it. Cheers, Chris Egon Willighagen wrote: > On Fri, Sep 5, 2008 at 12:26 AM, Stefan Kuhn <ste...@eb...> wrote: >> Dear co-developers, >> two comments which might be important for the upcoming release: >> 1. I fixed the ModelBuilder3d - all tests except one work (will look at this >> tomorrow). The reason were changes in the fingerprinter, > > Such as? I think Christoph regenerated fingerprints too, after which > the main FingerPrinter did no longer change... how come the index is > again broken then? (It's been puzzling me...) > >> apart from the >> ill-fated changes in the modelbuilder itself, which I fixed earlier. I can >> only ask everybody to run tests (I know I might have missed that myself). I >> added a test case to the TemplateHandler3DTest, which tests the fingerprints >> for stability. > > Please provide some information in what was/is causing fails... > >> 2. While working on this, I found the fingerprinter is always doing >> aromaticity detection. > > Mmmm... I'm not happy to hear that... this means that fingerprints not > only depend on code in the FingerPrinter, but in the > CDKHueckelAromatictyDetecter and the CDKAtomTypeMatcher too... even > bug fixes in these would make the fingerprints change... this *would* > explain why the fingerprints have been changing so often... > >> While I think we do not yet have a solution for "a >> detection if needed only", the class should at least have an option to switch >> this off. > > That idea pops up here too... > >> Worse, when running the ModelBuilder3DTests with a slight change, I >> get errors "org.openscience.cdk.exception.NoSuchAtomTypeException: The >> AtomType Csp2 could not be found". It looks like the aromaticity detection >> (now?) relies on atom typing, > > Sure... one needs to know if all atoms in the possibly aromatic ring > are sp2 or planar3 (N, O) and how many pi electrons they contribute to > the ring system. > >> but atom typing fails on aromatic carbons. > > In what way? With the above Csp2 atom type missing? Csp2 is not a CDK > atom type name... the CDKHueckelAromaticityDetector expects both CDK > atom type names and IAtomType.hybridization to be set properly... > >> This >> sounds alarming for me. If you want to test, change line 171 of >> TemplateHandler3D from BitSet ringSystemFingerprint = new >> Fingerprinter().getFingerprint(queryRingSystem); >> to BitSet ringSystemFingerprint = new >> Fingerprinter().getFingerprint(ringSystems); and run the ModelBuilder3D >> tests. four of them fail with this problem. > > Nasty problem... this more or less means that we have to update the > ring template index upon every change in CDKAtomTypeMatcher and > CDKHueckelAromaticityChecker... the latter less of a problem than the > first, which happens much more often... > > How many fingerprints do you now test? Can we rely on your new unit > test to 'notice' when we need to recalculate the index? > > Egon > -- Dr. Christoph Steinbeck Head of Chemoinformatics and Metabolism European Bioinformatics Institute (EBI) Wellcome Trust Genome Campus Hinxton, Cambridge CB10 1SD UK Phone +44 1223 49 2640 What is man but that lofty spirit - that sense of enterprise. ... Kirk, "I, Mudd," stardate 4513.3.. |