From: Christoph S. <ste...@ic...> - 2002-04-03 11:51:35
|
Oliver Horlacher wrote: > You need to initialize the atoms properly. The smiles you get is for an > isotope of mass 0. Good point :-) Just as an information for the cdk-devel list: Oliver Horlacher has written a canonical SmilesGenerator and I am in the process of comitting this to the CVS while resolving some minor problems on my side. One problem is that the SmilesGenerator requires (quite reasonably) properly configured Atom objects. Just as a reminder: Atom extends AtomType, which again extends Isotope. This is one problem of the CDK. There is no mechanism to get a properly configured atom when you use the "new Atom("C");" constructor. We have a "configure(Atom atom)" in the ElementFactory, but somehow it would be nicer to get a properly configured would be created by just the Atom(String symbol) constructor. For now I have made the configure(Atom atom) method, which so far returned void, return the atom, so that one can make molecule.addAtom(elFac.configure(new Atom("C"))); Do you have any suggestion on how one could sort this problem out a little nicer? Cheers, Chris -- Dr. Christoph Steinbeck (http://www.ice.mpg.de/departments/ChemInf) MPI of Chemical Ecology, Winzerlaer Str. 10, Beutenberg Campus, 07745 Jena, Germany Tel: +49(0)3641 571263 - Fax: +49(0)3641 571202 What is man but that lofty spirit - that sense of enterprise. ... Kirk, "I, Mudd," stardate 4513.3.. |
From: Egon W. <eg...@sc...> - 2002-04-04 05:33:07
|
On Wednesday 3 April 2002 13:49, Christoph Steinbeck wrote: > Oliver Horlacher wrote: > > You need to initialize the atoms properly. The smiles you get is for an > > isotope of mass 0. > > Good point :-) > > Just as an information for the cdk-devel list: > Oliver Horlacher has written a canonical SmilesGenerator and I am in the > process of comitting this to the CVS while resolving some minor problems > on my side. > > One problem is that the SmilesGenerator requires (quite reasonably) > properly configured Atom objects. Just as a reminder: Atom extends > AtomType, which again extends Isotope. > > This is one problem of the CDK. There is no mechanism to get a properly > configured atom when you use the "new Atom("C");" constructor. > > We have a "configure(Atom atom)" in the ElementFactory, but somehow it > would be nicer to get a properly configured would be created by just the > Atom(String symbol) constructor. > For now I have made the configure(Atom atom) method, which so far > returned void, return the atom, so that one can make > molecule.addAtom(elFac.configure(new Atom("C"))); > > Do you have any suggestion on how one could sort this problem out a > little nicer? I think the current method works fine... Indeed, for performance reasons a basic atom need not be configured with defaults... There are a few alternatives if Atom() remains as it is: 1. the helper class, like ElementFactory 2. a class DefaultAtoms with static fields, like CARBON which are fully preconfigured atoms 3. public class DefaultedAtom extends Atom which implements the Atom(String atomName) as a funtion where it is fully preconfigured by default I think the first one is pretty nice, though we use the second mechanism in CDK too (for other default data)... What/Why do you think (there) is something wrong with a helper class? Egon |
From: Christoph S. <ste...@ic...> - 2002-04-04 07:27:10
|
Egon Willighagen wrote: > On Wednesday 3 April 2002 13:49, Christoph Steinbeck wrote: > >>Oliver Horlacher wrote: >> >>>You need to initialize the atoms properly. The smiles you get is for an >>>isotope of mass 0. >> >>Good point :-) >> >>Just as an information for the cdk-devel list: >>Oliver Horlacher has written a canonical SmilesGenerator and I am in the >>process of comitting this to the CVS while resolving some minor problems >>on my side. >> >>One problem is that the SmilesGenerator requires (quite reasonably) >>properly configured Atom objects. Just as a reminder: Atom extends >>AtomType, which again extends Isotope. >> >>This is one problem of the CDK. There is no mechanism to get a properly >>configured atom when you use the "new Atom("C");" constructor. >> >>We have a "configure(Atom atom)" in the ElementFactory, but somehow it >>would be nicer to get a properly configured would be created by just the >>Atom(String symbol) constructor. >>For now I have made the configure(Atom atom) method, which so far >>returned void, return the atom, so that one can make >>molecule.addAtom(elFac.configure(new Atom("C"))); >> >>Do you have any suggestion on how one could sort this problem out a >>little nicer? > > > I think the current method works fine... Indeed, for performance reasons a > basic atom need not be configured with defaults... There are a few > alternatives if Atom() remains as it is: > > 1. the helper class, like ElementFactory > 2. a class DefaultAtoms with static fields, like CARBON which are fully > preconfigured atoms > 3. public class DefaultedAtom extends Atom > which implements the Atom(String atomName) as a funtion where it > is fully preconfigured by default > > I think the first one is pretty nice, though we use the second mechanism in > CDK too (for other default data)... What/Why do you think (there) is > something wrong with a helper class? Nothing's wrong in particular. It is just so nicely short to build molecules in testing applications like they are build in the MoleculeFactory (m = new Molecule(); m.addAtom(new Atom("C"));). If the Atoms were configured correctly in these few steps without additional code it would be great. Cheers, Chris -- Dr. Christoph Steinbeck (http://www.ice.mpg.de/departments/ChemInf) MPI of Chemical Ecology, Winzerlaer Str. 10, Beutenberg Campus, 07745 Jena, Germany Tel: +49(0)3641 571263 - Fax: +49(0)3641 571202 What is man but that lofty spirit - that sense of enterprise. ... Kirk, "I, Mudd," stardate 4513.3.. |
From: Egon W. <eg...@sc...> - 2002-04-04 18:58:45
|
On Thursday 4 April 2002 09:25, Christoph Steinbeck wrote: > Egon Willighagen wrote: > > On Wednesday 3 April 2002 13:49, Christoph Steinbeck wrote: > >>Oliver Horlacher wrote: > >>>You need to initialize the atoms properly. The smiles you get is for an > >>>isotope of mass 0. > >> > >>Good point :-) > >> > >>We have a "configure(Atom atom)" in the ElementFactory, but somehow it > >>would be nicer to get a properly configured would be created by just the > >>Atom(String symbol) constructor. > >>For now I have made the configure(Atom atom) method, which so far > >>returned void, return the atom, so that one can make > >>molecule.addAtom(elFac.configure(new Atom("C"))); > >> > >>Do you have any suggestion on how one could sort this problem out a > >>little nicer? > > > > I think the current method works fine... Indeed, for performance reasons > > a basic atom need not be configured with defaults... There are a few > > alternatives if Atom() remains as it is: > > > > 1. the helper class, like ElementFactory > > 2. a class DefaultAtoms with static fields, like CARBON which are fully > > preconfigured atoms > > 3. public class DefaultedAtom extends Atom > > which implements the Atom(String atomName) as a funtion where it > > is fully preconfigured by default > > > > I think the first one is pretty nice, though we use the second mechanism > > in CDK too (for other default data)... What/Why do you think (there) is > > something wrong with a helper class? > > Nothing's wrong in particular. It is just so nicely short to build > molecules in testing applications like they are build in the > MoleculeFactory (m = new Molecule(); m.addAtom(new Atom("C"));). If the > Atoms were configured correctly in these few steps without additional > code it would be great. Then a class with static field would be best... Then you could do MoleculeFactory (m = new Molecule(); m.addAtom(DefaultAtoms.CARBON);) You would loose then the flexibility with the source of the atom type... it is then hard-coded... whereas in you case, the "C" part can be taken from file... Egon |
From: Christoph S. <ste...@ic...> - 2002-04-16 14:19:07
Attachments:
pinene.gif
|
Dear Oliver, dear Co-Coders, There might a bug in the SMILES generator. I was testing the generator with my favorite molecule a-pinene. The code for generating the molecule was: public static Molecule makeAlphaPinene() { Molecule mol = new Molecule(); mol.addAtom(new Atom("C")); // 1 mol.addAtom(new Atom("C")); // 2 mol.addAtom(new Atom("C")); // 3 mol.addAtom(new Atom("C")); // 4 mol.addAtom(new Atom("C")); // 5 mol.addAtom(new Atom("C")); // 6 mol.addAtom(new Atom("C")); // 7 mol.addAtom(new Atom("C")); // 8 mol.addAtom(new Atom("C")); // 9 mol.addAtom(new Atom("C")); // 10 mol.addBond(0, 1, 2); // 1 mol.addBond(1, 2, 1); // 2 mol.addBond(2, 3, 1); // 3 mol.addBond(3, 4, 1); // 4 mol.addBond(4, 5, 1); // 5 mol.addBond(5, 0, 1); // 6 mol.addBond(0, 6, 1); // 7 mol.addBond(3, 7, 1); // 8 mol.addBond(5, 7, 1); // 9 mol.addBond(7, 8, 1); // 10 mol.addBond(7, 9, 1); // 11 configureAtoms(mol); return mol; } The hydrogen count was correctly assigned (see attached picture): [java] Hydrogen count for atom 0: 0 [java] Hydrogen count for atom 1: 1 [java] Hydrogen count for atom 2: 2 [java] Hydrogen count for atom 3: 1 [java] Hydrogen count for atom 4: 2 [java] Hydrogen count for atom 5: 1 [java] Hydrogen count for atom 6: 3 [java] Hydrogen count for atom 7: 0 [java] Hydrogen count for atom 8: 3 [java] Hydrogen count for atom 9: 3 The resulting SMILES is: [java] [java] SMILES 2: c1c(C)c2cc(c1)C2(C)(C) It seems that some of the atoms are regarded as aromatic which they clearly aren't :-) The code is in the CVS (if you are faster than the next comitter, then you can just execute the "run" ant target of the current CVS state.) Run SmilesGeneratorTest. Cheers, Chris -- Dr. Christoph Steinbeck (http://www.ice.mpg.de/departments/ChemInf) MPI of Chemical Ecology, Winzerlaer Str. 10, Beutenberg Campus, 07745 Jena, Germany Tel: +49(0)3641 571263 - Fax: +49(0)3641 571202 What is man but that lofty spirit - that sense of enterprise. ... Kirk, "I, Mudd," stardate 4513.3.. |
From: E.L. W. <eg...@sc...> - 2002-04-16 14:51:12
|
On Tuesday 16 April 2002 16:17, Christoph Steinbeck wrote: > Dear Oliver, dear Co-Coders, > > There might a bug in the SMILES generator. I was testing the generator > with my favorite molecule a-pinene. The code for generating the molecule > was: Do we have an implementation of a connectivity table? And a library that does matrix operations like multiplication? If so, I will write a aromaticity detector tomorrow... Egon PS. Do you like the keyword index on the webpage? http://cdk.sourceforge.net/keywordindex.html |
From: Christoph S. <ste...@ic...> - 2002-04-16 15:38:08
|
E.L. Willighagen wrote: > On Tuesday 16 April 2002 16:17, Christoph Steinbeck wrote: > >>Dear Oliver, dear Co-Coders, >> >>There might a bug in the SMILES generator. I was testing the generator >>with my favorite molecule a-pinene. The code for generating the molecule >>was: > > > Do we have an implementation of a connectivity table? > And a library that does matrix operations like multiplication? > > If so, I will write a aromaticity detector tomorrow... > > Egon > > PS. Do you like the keyword index on the webpage? > > http://cdk.sourceforge.net/keywordindex.html The keyword index is great. Thanks for pointing the attention to it. How rigorous is this aromaticity detection based on matrix multiplication? Does it only do benzene (I believe so) or can it handle systems like azulene and can it correctly handle contributions from free electron pairs, like from the nitrogen in indole? I have ported the aromaticity detector from the compchem classes. It is based on the AllRingsFinder which has also been ported. The AromaticityDetector still needs debugging, though. The problem with matrix multiplications is also that they scale horribly (O(N^3) or so). Cheers, Chris -- Dr. Christoph Steinbeck (http://www.ice.mpg.de/departments/ChemInf) MPI of Chemical Ecology, Winzerlaer Str. 10, Beutenberg Campus, 07745 Jena, Germany Tel: +49(0)3641 571263 - Fax: +49(0)3641 571202 What is man but that lofty spirit - that sense of enterprise. ... Kirk, "I, Mudd," stardate 4513.3.. |
From: Egon W. <eg...@sc...> - 2002-04-16 17:25:53
|
On Tuesday 16 April 2002 17:36, Christoph Steinbeck wrote: > E.L. Willighagen wrote: > > On Tuesday 16 April 2002 16:17, Christoph Steinbeck wrote: > The problem with matrix multiplications is also that they scale horribly > (O(N^3) or so). Not sure. I'll have to see... I do not have time in the next few weeks to dig up related articles... but will do some testing on a few molecules... Egon |
From: Christoph S. <ste...@ic...> - 2002-04-16 15:27:22
|
Oliver Horlacher wrote: > Aye there is a bug. I forgot to check for the 2 e- in a ring which > gives 0%4 which is 0 and by the old code makes it aromatic. If you > change line 75 to > if(eCount - 2 != 0 && (eCount - 2)%4 == 0) > > that bug will be fixed. > > Cheerers > > Olli > Great, works. Bug fixed. The debugged code is in the CVS. Thanks. Chris -- Dr. Christoph Steinbeck (http://www.ice.mpg.de/departments/ChemInf) MPI of Chemical Ecology, Winzerlaer Str. 10, Beutenberg Campus, 07745 Jena, Germany Tel: +49(0)3641 571263 - Fax: +49(0)3641 571202 What is man but that lofty spirit - that sense of enterprise. ... Kirk, "I, Mudd," stardate 4513.3.. |