[Cdk-user] Clarification about LargestChainDescriptor behavior

2016-02-11 Thread Giovanni Cincilla
Dear all, I use CDK mainly through KNIME and I found some supposed inconsistencies using the LargestChainDescriptor. I originally posted my doubt in KNIME-CDK forum where I also provided examples: https://tech.knime.org/forum/cdk/cdk-largest-chain-descriptor-inconsistencies#comment-41502 I'm not

Re: [Cdk-user] Clarification about LargestChainDescriptor behavior

2016-02-11 Thread Rajarshi Guha
Could you provide an example where issues 2 & 3 show up? The descriptor is meant to compute the length of the longest aliphatic chain in a molecule On Thu, Feb 11, 2016 at 4:29 AM, Giovanni Cincilla wrote: > Dear all, > I use CDK mainly through KNIME and I found some

Re: [Cdk-user] Clarification about LargestChainDescriptor behavior

2016-02-11 Thread Rajarshi Guha
Yes, it does appear to be a bug. Inspeting the code the LargestChainDescriptor is looking for the longest path that contains non-aromatic, non-ring atoms. For Row0, there are only two non-ring, non-aromatic atoms and so there are two possible chains with a single atom, hence each chain has the

Re: [Cdk-user] Clarification about LargestChainDescriptor behavior

2016-02-11 Thread Rajarshi Guha
you can ignore the patch since I've just made a pull request with this fix On Thu, Feb 11, 2016 at 10:47 AM, Rajarshi Guha wrote: > Yes, it does appear to be a bug. > > Inspeting the code the LargestChainDescriptor is looking for the longest > path that contains

Re: [Cdk-user] Clarification about LargestChainDescriptor behavior

2016-02-11 Thread John M
Cheers for patch Rajarshi, added some clean up the code: https://github.com/cdk/cdk/pull/195 but get a test failure on, "CC=CC(C)=O" expected longest path is 6 but I get 5? 5 looks correct to me? John Regards, John W May john.wilkinson...@gmail.com On 11 February 2016 at 17:03, Rajarshi Guha

Re: [Cdk-user] Clarification about LargestChainDescriptor behavior

2016-02-11 Thread Rajarshi Guha
well the first looks at all atoms, the other only considers carbon chains. I'd say keep them both in for now (though the first could be parametrized on element, in which case we'd only need one) On Thu, Feb 11, 2016 at 1:06 PM, John M wrote: > Hmm.. should we