Re: [Cdk-devel] [Cdk-commits] SF.net SVN: cdk: [11547] cdk/branches/drzz-jumbo5.4

[Daniel Z. <zah...@dt...>]

On Jul 12, 2008, at 4:15 AM, Egon Willighagen wrote:

> Hi Dan,
>
> On Fri, Jul 11, 2008 at 4:42 PM,  <dr...@us...> wrote:>
> @@ -133,7 +133,7 @@
>>                        CMLAtomArray ar=new CMLAtomArray();
>>                        for (int i=0; i<chargeGroup.getAtomCount();i 
>> ++){
>>                                CMLAtom cmlAtom=new CMLAtom();
>> -                               cmlAtom.setRef(chargeGroup.getAtom 
>> (i).getID());
>> +                               cmlAtom.setId(chargeGroup.getAtom 
>> (i).getID());
>>
>>                                //Append switching atom?
>>                                if (chargeGroup.getAtom(i).equals 
>> (chargeGroup.getSwitchingAtom())) {
>
> This change should have better been submitted as separate change, as
> it actually changes the way to code behaves... I assume it fixes an
> incorrect use of CML, which is fine of course, but does deserve at
> least a commit message, but really a unit test too, to ensure that the
> same mistake is not made again...

OK, maybe I'll just add some comments and commit those changes. I'm  
not sure what unit test would add anything. The reason the changes  
were made is that 5.4 throws an exception when trying to add an atom  
with no id. That was already caught by the present unit test. I'll  
have to think about other things that may need new unit tests to pick  
up.
>
> Interestingly, the use of @ref might be correct in certain CML uses,
> where later atoms are actually refering to earlier definitions... if
> not mistaken, this might be used for certain reactions...

Yes, this gets into what I mentioned in the other reply about CML  
issues to work on. The code is for interfacing to a molecular  
dynamics program (unnamed, see http://wiki.cubic.uni-koeln.de/cdkwiki/ 
doku.php?id=mdmolecule). The current code is certainly internally  
consistent (that's exactly what the unit test checks), but is it  
consistent with broader CML usage? The main goals are to code residue  
and chargeGroup membership. It uses submolecules for this:

<molecule>
   <atomArray>
     <atom id="a1">
      ...
    </atomArray>

    <molecule distRef="md:residue"  id="r1" titel="Rname">
      <atomArray>
        <atom ref="a1">
     </atomArray>
    </molecule>

</molecule>

I seem to recall that there were more direct ways to code this in  
CML, but I need to check with the Cambridge people. There's also the  
question of exactly what the difference in usage between ref and id  
and how that applies to the present situation. Is an atom in the  
submolecule a new distinct atom which uses a ref to connect to the  
"real" atom it refers to or is it a "copy" of the "real atom" which  
can be indicated by using the same id? I don't think these questions  
are really Jumbo 5.4 specific and I would really like to get Jumbo  
5.4 in and then start working on these issues rather than try to take  
care of it all as part of the 5.4 transition. As I type this, it  
occurs to me the minimal change which should keep compatibility with  
any use of this code would be to write both an id and a ref, so in  
the fragment above in the residue molecule:

   <atom ref="a1" id ="r1_a1">

I think this should get us to a workable state.



>
> Maybe the best way to resolve this 'issue' is to see who commited that
> setRef() line, and ask why setRef() was used there...
>
> You can use 'svn annotate' to see who last changed that line...
>

It looks like the previous code was added by you in Revision 7971 in  
Feb 2007.  Do you know what the MD program is and is anyone still  
using it?

DanZ


/********************************************
    Daniel Zaharevitz
    Chief, Information Technology Branch
    National Cancer Institute
    zah...@dt...
********************************************/