Re: [Cdk-devel] NumericalSurface returns different results after rigid alignment

[John M. <joh...@gm...>]

Not tested but I think the problem is here:
https://github.com/cdk/cdk/blob/master/descriptor/qsarmolecular/src/main/java/org/openscience/cdk/geometry/surface/NeighborList.java#L55-L61

Shouldn't the else also add the atom to the list?

A shorter, more efficient and less error prone idiom is:

List<IAtom> atoms = this.boxes.get(key);
> if (atoms == null)
>   this.boxes.put(key, atoms = new ArrayList<>());
> atoms.add(atom);


Notice we do a single get, then iff the value was null we create a new one
and add it. We always add the atom to the atoms array.

I can see that some people may find the assignment in the put confusing (in
C++ the put would do a copy so the atoms.add looks like a no-op).

John

On 20 February 2017 at 13:57, Rajarshi Guha <raj...@gm...> wrote:

> Here's some sample code https://gist.github.com/rajarshi/
> 86389392173191ee15cef7b76d994ae5 and the SD files are available at
> https://www.dropbox.com/sh/xu548wx8brqx9z9/AABloecWNrSXAEcwud7ZSV-Oa?dl=0
>
> Poking around I found our that the neighbor lists for a given atom differ
> between the aligned and non-aligned cases.
>
> On Mon, Feb 20, 2017 at 4:59 AM, John Mayfield <
> joh...@gm...> wrote:
>
>> Have you got the molecule/usage? My money is on either order of
>> operations / Tesselate.java.
>>
>> John
>>
>> On 20 February 2017 at 02:54, Rajarshi Guha <raj...@gm...>
>> wrote:
>>
>>> Hi, a user pointed out a bug in the CPSA descriptor that is generating
>>> different results for the same molecule after a rigid alignment.
>>>
>>> It turns out that the bug arises due to NumericalSurface giving
>>> different total surface area values for the molecule before alignment and
>>> after alignment.
>>>
>>> I confirmed that the relative positions of the atoms are identical in
>>> the two versions of the molecule (pairwise distance matrix is the same to
>>> four decimal places).
>>>
>>> Does anybody have any immediate ideas about why a rigid alignment would
>>> lead to a difference in the computed surface area?
>>>
>>> --
>>> Rajarshi Guha | http://blog.rguha.net
>>> NIH Center for Advancing Translational Science
>>>
>>> ------------------------------------------------------------
>>> ------------------
>>> Check out the vibrant tech community on one of the world's most
>>> engaging tech sites, SlashDot.org! http://sdm.link/slashdot
>>> _______________________________________________
>>> Cdk-devel mailing list
>>> Cdk...@li...
>>> https://lists.sourceforge.net/lists/listinfo/cdk-devel
>>>
>>>
>>
>
>
> --
> Rajarshi Guha | http://blog.rguha.net
> NIH Center for Advancing Translational Science
>