I may be wrong here, but my impression is that both OBMol and OBAtom =
operators work fine, the problem is that formal charges are zeroed after
the call to Clear() in the OBMol = operator, as can ve verified printing
the results of GetFormalCharge() before and after Clear(). Therefore
what OBAtom::Duplicate duplicates are actually zeros, except for the +2
charge that the atom typer is able to assign. Apparently, the atom typer
can assign some formal charges (e.g., in your example the +2 charge of
one of the Fe ions), but some of them need to be read from user input,
since they cannot be univocally determined.
I had this problem to assign MMFF94 atom types to Cu2+and Cu+. There is
no way the atom typer can understand if you have Cu(I) or Cu(II), so
correct MMFF94 requires formal charge input by the user, such as the one
supplied in SDF files.
It could be that I'm completely wrong, since I don't have a
comprehensive picture of the way OB works, I'm just telling you about my
experience, so maybe someone can find a better patch than mine (which
works, though :-)
Cheers,
p.
On 05/17/2012 08:00 PM, Craig James wrote:
I have narrowed down the "lost charge" problem to atomtyper.cpp.
OBMol::operator= calls OBAtom::operator=, which calls the atom typer
to figure out the type of the *source* atom. During that process, the
atom typer erases the charge on the source atoms.
I'm still digging, but I'm way beyond my area of familiarity with the
OB code. Paolo's patch is a workaround, but this is a pretty serious
bug in the OBAtom::operator=.
Craig
On Thu, May 17, 2012 at 9:38 AM, Craig James <cja...@emolecules.com
<mailto:cja...@emolecules.com>> wrote:
On Thu, May 17, 2012 at 9:04 AM, Chris Morley
<c.mor...@gaseq.co.uk <mailto:c.mor...@gaseq.co.uk>> 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?
Excellent point. It turns out the SMILES format copies the
molecule before generating the SMILES. The "=" operator seems to
be the culprit. I modified smilesformat.cpp as follows:
bool SMIBaseFormat::WriteMolecule(OBBase* pOb,OBConversion* pConv)
{
//cout << "SMIBaseFromat::WriteMolecule()" << endl;
OBMol* pmol = dynamic_cast<OBMol*>(pOb);
FOR_ATOMS_OF_MOL(atom, pmol) {
fprintf(stderr, "smilesformat: atom %d: formal charge: %d,
spin mult: %d\n",
atom->GetIdx(), atom->GetFormalCharge(),
atom->GetSpinMultiplicity());
}
// Define some references so we can use the old parameter names
ostream &ofs = *pConv->GetOutStream();
OBMol mol = *pmol;
FOR_ATOMS_OF_MOL(atom, mol) {
fprintf(stderr, "smilesformat: atom %d: formal charge: %d,
spin mult: %d\n",
atom->GetIdx(), atom->GetFormalCharge(),
atom->GetSpinMultiplicity());
}
Here's what it prints. Simply copying the molecule with the "="
operator loses the charges.
smilesformat: atom 1: formal charge: 2, spin mult: 0
smilesformat: atom 2: formal charge: -2, spin mult: 0
smilesformat: atom 3: formal charge: 2, spin mult: 0
smilesformat: atom 4: formal charge: -2, spin mult: 0
smilesformat: atom 1: formal charge: 2, spin mult: 0
smilesformat: atom 2: formal charge: 0, spin mult: 0
smilesformat: atom 3: formal charge: 0, spin mult: 0
smilesformat: atom 4: formal charge: 0, spin mult: 0
I'll keep digging... but if anyone recalls making a change that's
related to this, we could zero in faster.
Thanks,
Craig
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>
> <mailto: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
<mailto:OpenBabel-discuss@lists.sourceforge.net>
https://lists.sourceforge.net/lists/listinfo/openbabel-discuss
------------------------------------------------------------------------------
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
