Dear Chris, you are right that my patch seemingly duplicates the copying of atom formal charges which is already done in the OBAtom::Duplicate(); however, OBAtom::Duplicate() comes after Clear() has been called, so formal charges have been reset. I have verified that, and this is why my patch works, "saving" formal charges before calling Clear(). I experienced the same problem with MMFF94 typing; some functionality needs pre-set formal charges, such as Craig's case and mine.
Cheers, p. On 05/17/2012 06:04 PM, Chris Morley wrote: > Craig's failing example works fine for me under Windows with both 2.3.1 > and the development code. As he suggested, this inconsistency suggests > that there is something not being initialized properly. The logic around > the flag OB_TCHARGE_MOL is a bit tricky and so is a candidate, but I > cannot see anything wrong. Paolo's patch is in the same area in the > OBMol assignment operator (which is used in SMILES output) but as far as > I can see it just duplicates the copying of atom formal charges which is > already done in the OBAtom::Duplicate(). I think it would be better to > identify the fault before applying the patch. But it is tricky finding a > bug if you cannot reproduce it. Can anyone help? > > Chris > > On 17/05/2012 16:28, Craig James wrote: >> Hi Paolo, >> >> Thanks very much for the patch. Unfortunately, the OBForceField >> functions that you patched are never called at all during the >> SDF-to-SMILES conversion process, so the patch doesn't resolve my >> issue. OBForceField only seems to be called if I use the --gen3D option. >> >> Best regards, >> Craig >> >> On Thu, May 17, 2012 at 3:03 AM, Paolo Tosco<paolo.to...@unito.it >> <mailto:paolo.to...@unito.it>> wrote: >> >> Dear Craig, >> >> I have faced the identical problem with MMFF94, and I have >> previously submitted a similar patch which has already been applied >> to SVN OpenBabel. However, now I realize that my fix had only solved >> my own problem, but not a more general problem connected with formal >> charges. >> So I have attached a much better patch, which reverts my previous >> one, and instead applies it on the OBMol copy constructor, so that >> it becomes of general use. >> >> To make a long story short, if you patch the current OpenBabel SVN >> version with the attached patch your issue will be solved. >> >> From the OpenBabel root folder: >> >> patch -p0< patch_17-05-2012.patch >> >> Dear Geoff, would you please apply it also to the SVN trunk so that >> also other OpenBabel users may benefit from that? It also includes a >> small patch on "A T O M T Y P E S" logging that I had left behind >> previously. >> >> Best regards, >> Paolo >> >> >> On 05/17/2012 01:44 AM, Craig James wrote: >>> Here's another problem in 2.3.x that's not in 2.2.x. I realize >>> these are bogus molecules, but it illustrates a serious problem. >>> >>> $ echo "[Fe+2][Fe-2].[Fe+2].[Fe-2]" | babel -i smi -o can >>> [Fe-2][Fe+2].[Fe-2].[Fe+2] >>> >>> Everything looks good -- charges are maintained. Now go through >>> the SDF format: >>> >>> $ echo "[Fe+2][Fe-2].[Fe+2].[Fe-2]" | babel -i smi -o sdf | >>> babel -i sdf -o can >>> [Fe+2][Fe].[Fe].[Fe] >>> >>> Not so good -- charges are dropped. Where is the problem? It >>> turns out the SDF is correct: >>> >>> $ echo "[Fe+2][Fe-2].[Fe+2].[Fe-2]" | babel -i smi -o sdf >>> >>> OpenBabel05161216052D >>> >>> 4 1 0 0 0 0 0 0 0 0999 V2000 >>> 0.0000 0.0000 0.0000 Fe 0 2 0 0 0 0 0 0 0 0 0 0 >>> 0.0000 0.0000 0.0000 Fe 0 6 0 0 0 0 0 0 0 0 0 0 >>> 0.0000 0.0000 0.0000 Fe 0 2 0 0 0 15 0 0 0 0 0 0 >>> 0.0000 0.0000 0.0000 Fe 0 6 0 0 0 15 0 0 0 0 0 0 >>> 1 2 1 0 0 0 0 >>> M CHG 4 1 2 2 -2 3 2 4 -2 >>> M END >>> $$$$ >>> >>> The charges are all properly marked in the atom block and in the M >>> CHG line. And the "15" in atoms 3 and 4 means "zero valence" >>> which is right. Yet the SMILES is wrong. >>> >>> It turns out this same thing happens if you use >>> atom->SetFormalCharge(). It sometimes works and sometimes doesn't. >>> >>> Note that it works correctly in OpenBabel 2.2.x: >>> >>> $ echo "[Fe+2][Fe-2].[Fe+2].[Fe-2]" | babel -i smi -o sdf | >>> babel -i sdf -o can >>> [Fe-2].[Fe-2][Fe+2].[Fe+2] >>> >>> Anyone have any idea what changed, either in the OpenBabel >>> internals or the SMILES generator, that would affect the way >>> charges are interpreted? >>> >>> Craig > ------------------------------------------------------------------------------ > Live Security Virtual Conference > Exclusive live event will cover all the ways today's security and > threat landscape has changed and how IT managers can respond. Discussions > will include endpoint security, mobile security and the latest in malware > threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/ > _______________________________________________ > OpenBabel-discuss mailing list > OpenBabel-discuss@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/openbabel-discuss > -- ========================================================== Paolo Tosco, Ph.D. Phone: +39 011 6707680 Department of Drug Science Fax: +39 011 6707687 and Technology Mob: +39 348 5537206 Via Pietro Giuria, 9 10125 Torino, Italy http://open3dalign.org E-mail: paolo.to...@unito.it http://open3dqsar.org ========================================================== ------------------------------------------------------------------------------ Live Security Virtual Conference Exclusive live event will cover all the ways today's security and threat landscape has changed and how IT managers can respond. Discussions will include endpoint security, mobile security and the latest in malware threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/ _______________________________________________ OpenBabel-discuss mailing list OpenBabel-discuss@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openbabel-discuss
