#489 fix bcuts for undefined burden matrix values

Needs_Review
closed
cdk-1.4.x (181)
5
2012-10-28
2012-02-20
Rajarshi Guha
No

Addresses bug 3489559 - slightly different, in that the cause of the problem is not invertability, but undefined values in the Burden matrix. Attached patch fixes this

Discussion

  • A few minor suggested things:

    • @TestMethod(value = "testNamesConsistency") can be just @TestMethod("testNamesConsistency")

    • return getDummyDescriptorValue(new CDKException("Burden matrix has undefined values"));

    Can you add some source code comment there about what could cause these undefined values? Wrong input? Would it make sense to test the input structure earlier for things that cause undefined values?

    • ChemFile content = (ChemFile) reader.read((ChemObject) new ChemFile());

    That can be simplified to:

    ChemFile content = reader.read(new ChemFile());

    All requests are minor suggestions, to be applied at the discretion of the author.

     
  • Rajarshi Guha
    Rajarshi Guha
    2012-02-20

    Updated patch. I left out the code comment, because I really don't know why the partial charge code leads to NaN's (as far as I can tell other burden matrices won't have NaNs)

     
  • Can you perhaps create a unit test that shows that partial charges are not correctly calculated for this compound then?

    Meanwhile, I'll look at the updated patch...

     
  • Rajarshi Guha
    Rajarshi Guha
    2012-02-20

    Added a unit test for the G-M failure

     
  • Thanks!