#118 Re-write of the atom container permutation classes

Needs_Review
closed
master (162)
5
2012-10-28
2009-10-18
No

The attached patches do two things:

1) Add a class called "Permutor" that generates arrays of ints like "[0,2,1]"
2) Re-writes the AtomContainer[Atom|Bond]Permutor classses to use this

The reason for doing this is mainly for testing, as the provided
permutation generator is more functionally rich. There are N!
permutations of N things, so a molecule with 10 atoms has 10! or
3,628,800 permutations while one with 20 atoms has
2,432,902,008,176,640,000 - clearly far too many to test. What can be
done with the Permutor class is randomly sample permutations by simply
skipping ahead in the list.

In addition, the ranking/unranking method that the class uses means it
is possible to go to a particular permutation very easily. This is
explained to some extent in the comments of the class.

The commit hash is :

commit 92b9bf22c1c06b33fb03dafe81715f04d37881f8

if that helps.

Discussion

  • Egon Willighagen

    I am applying this patch, and currently doing several fixes...

    What I noticed so far:

    • use @cdk.cite and cheminf.bibx for references
    • unit tests!
    • missing unit test annotation
    • improper copyright statement (do not use CDK Project)
     
  • Egon Willighagen

    Hi Gilleain,

    JavaDoc seems mostly fine, PMD I have not checked yet, but please add unit tests first. I have updated your original two patches to compile against master to compensate for the ILoggingTool patch, and added two patches with template code for you to complete:

    0003: please add the year, publisher, ISBN and more if possible
    0004: framework for the unit testing

     
  • gilleain maclean torrance

    After a mere two years, finally added unit tests. The original AtomContainerPermutorTest was in test-extra for some reason, so moved to test-standard. Also added ISBN, and a few other small things. Permutor class is fully tested.

     
  • Egon Willighagen

    Thanx. Some minor points which you might be able to fix:

    • use @cdk.cite for cdk/graph/Permutor for the book citation
    • 'update' the copyright lines in the test classes, which now have:

    +/* $RCSfile$
    + * $Author$
    + * $Date$
    + * $Revision$

    These lines are from SVN time and can be removed.

      • Copyright (C) 1997-2007 The Chemistry Development Kit (CDK) project

    These are new files, right? Please use the year when created and add your name instead of 'The CDK project' which we should have never done :)

    Patches are applied, please consider this minor 'bug fixes', and fix at your convenience. I'll bitch it when we get to the next stable release anyway :)

     

Log in to post a comment.

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:

JavaScript is required for this form.





No, thanks