Re: [Rdkit-devel] I found Bug

[Greg L. <gre...@gm...>]

Dear Akihiro,


On Thu, Jul 16, 2009 at 2:18 PM, Akihiro
Yokota<aki...@ya...> wrote:
>
> Nice to meet you. I am a japanese researcher for computational chemistry,
> and I am newly studying programming of cheminformatics. Therefore I am
> studying RDKit code. I am very impressed by your effective, exciting, and
> beautiful programs. I hope you go to continue to develop RDKit in a long
> time.

Thank you for your kind words.

>
> And what I would like to say is I found a bug in RDKit. please see around
> 203th-line of RDKit_Q22009_1/Code/GraphMol/Depictor/EmbeddedFrag.cpp.
>
> ----------Orignal--------------
> d_eatoms[aid].nbr1 = nb1;
> d_eatoms[aid].nbr1 = nb2;
> -------------------------------
>
> ----------Correct?-------------
> d_eatoms[aid].nbr1 = nb1;
> d_eatoms[aid].nbr2 = nb2;
> -------------------------------

Very nicely found. Thank you. I will fix it and check it in.

>
> This code makes a bug in case of the following (rare) case. I found it when
> I am reading RDKitQ1_2009, but I think it is not be fixed yet in
> RDKitQ2_2009.
>
> When 4 atoms(e.g. A, B, C and E below) are already assigned their
> coordinates, and will add the coordinate of D, RDKit makes invalid results.
> (This is because nb2 is not changed from nb2=0.)
>
> (And I found that you changed the initial value of nb2=0 instead of nb2=-1
> in RDKitQ1_2009. I think nb2=-1 is preferred rather than nb2=0)
>
>   B
>   |
>   |
> A--E---C
>   |
>   |
>   D
>

This second part I'm going to have to think about a bit more before I
make the change.

Thanks for the detailed bug report (and suggested fix!).

-greg