Hi all,
I've been playing with joelib - cdk interaction recently and found the
following bug in org.openscience.cdk.libio.joelib.Convertor.java class. It
seems related to the unset atom properties issue, this time this is
atomicNumber property.
Basically, the convertor generates JoeMol object, but fails to assign the
correct atom types and instead assigns "Xx" as atom types.
The two-way test in CDK doesn't fail, since the test molecule has only carbon
atoms and during the backward conversion, atoms are assigned "C" atom type by
default.
This test works:
public void testCDKJoeMolAllC() {
Molecule mol = new Molecule();
mol.addAtom(new Atom("C")); // 0
mol.addAtom(new Atom("C")); // 1
mol.addBond(0, 1, 1); // 1
JOEMol converted = Convertor.convert(mol);
Molecule reverted = Convertor.convert(converted);
assertEquals(mol.getAtomCount(), reverted.getAtomCount());
assertEquals(mol.getBondCount(), reverted.getBondCount());
try {
IsomorphismTester it = new IsomorphismTester(mol);
assertTrue(it.isIsomorphic(reverted));
} catch (NoSuchAtomException e) {
assertTrue(false);
}
}
but this does not:
public void testCDKJoeMol() {
Molecule mol = new Molecule();
mol.addAtom(new Atom("C")); // 0
mol.addAtom(new Atom("O")); // 1
mol.addBond(0, 1, 1); // 1
JOEMol converted = Convertor.convert(mol);
Molecule reverted = Convertor.convert(converted);
assertEquals(mol.getAtomCount(), reverted.getAtomCount());
assertEquals(mol.getBondCount(), reverted.getBondCount());
try {
IsomorphismTester it = new IsomorphismTester(mol);
assertTrue(it.isIsomorphic(reverted));
} catch (NoSuchAtomException e) {
assertTrue(false);
}
}
The problem seems to be in
public static JOEAtom convert(Atom atom, int coordType) {
....
convertedAtom.setAtomicNum(atom.getAtomicNumber());
}
where type of the atom in JOEMol relies on atom.getAtomicNumber(), which is
not initialized in the tests above.
If the atomicNumber property are explicitely initialized before dowing the
conversion, it works properly.
Of course a quick fix in the Convertor (like below) is possible, but I am not
sure where is the right place for such initialization in general?
org.openscience.cdk.config.IsotopeFactory isotopeFactory =
org.openscience.cdk.config.IsotopeFactory.getInstance(atom.getBuilder());
isotopeFactory.configure(atom);
convertedAtom.setAtomicNum(atom.getAtomicNumber());
Best regards,
Nina
Egon Willighagen <[EMAIL PROTECTED]> wrote:
>
> RFC= request for comments
>
> Hi all,
>
> what is the hydrogen count when I type:
>
> IAtom atom = new IAtom("C");
>
> or, what happens when I have an atom container with 5 atoms, and I set the
> H-count for only one of them. Are the values zero for the other four then?
>
> The JavaDoc is not helpful here, though non-native fields simply default to
> null. I tended to say that if a property is not explicitly set, it is
> undefined, which matches the 'null' nicely. Native fields (int, double, etc)
> do not have a null concept, and some work by Rajarshi yesterday addressed
> this by explicitly stating a field was UNSET, by using Integer.MIN_VALUE and
> Double.NaN.
>
> This broke about 300 JUnit tests.
>
> However, too many algorithms (atom typing, canonical labeler, to name some)
> *do* expect a 0 default. For example, expect the above carbon to have zero
> formal charge, and zero implicit hydrogens. But I am sure that other
> algorithms expect something else.
>
> I made a brief attempt to fix some code here and there to deal with unset
> values, but I never had any chance... and I had to revert the changes and
> make formalCharge and hydrogenCount now explicitly default to zero.
>
> So, very importantly: what *are* the default values for all non-native
fields
> in the interfaces?! This needs to be defined *and* clearly stated in the
API.
> And if we go the UNSET route, this will require a whole lot of
implementation
> fixing!
>
> Egon
-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
Cdk-user mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/cdk-user
