Share

The Chemistry Development Kit

Tracker: Patches

5 Patches for CrossoverMachine and PartialFilledStructureMerge - ID: 2839472
Last Update: Comment added ( steinbeck )


Please find attached new implementations for CrossoverMachine and
PartialFilledStructureMerger and tests for them.
Both classes were untested and had no chance of ever working. The
CrossoverMachine could not return any result, as one could see from a short

glance at the code. The PartialFilledStructureMerger was a near copy of the

SingleStructureRandomGenerator and therefore also not doing what it was
supposed to do. Both classes are largely rewritten from scratch.


Stefan Kuhn ( shk3 ) - 2009-08-18 09:05

5

Open

None

Nobody/Anonymous

None

Needs Revision

Public


Comments ( 4 )

Date: 2009-08-26 13:20
Sender: steinbeckSourceForge.net SubscriberProject Admin

I assume that failure means that the algorithm is not able to saturate the
incomplete structure property. We have had this problem in all earlier
versions of this algorithm and unfortunately I'm not aware of any algorithm
that guarantees to saturate the structure properly in all cases. That
means: we can either remove this package from the chemistry development Kit
completely or we document the behavior and leave it in. Personally, I would
prefer the latter. Having a genetic algorithm optimization in the CDK is
important and it might attract users with more experience who might even be
able to fix the problem.



Date: 2009-08-26 12:31
Sender: shk3Project Admin

Fixing the complexity shouldn't be a problem, but the other stuff is. If I
would know what exactly are the reasons for failure it would not fail. The
judge use of the algorithm is up to the user, I would think. For example,
in many cases, you might just want to use the parents unchanged.


Date: 2009-08-18 11:21
Sender: egonwProject AdminAccepting Donations

Thanks for adding the unit test.

The other two comments I made, I do not find addressed sufficiently.

* Please explain what the source of those 299 fails are, and how that
affects the use of the algorithm.
* The old code was written before the project instantiated those rules.
New code should abide them, and I think you are qualified enough to write
clean code... your argument for not doing so, is not good enough for me.



Date: 2009-08-18 11:13
Sender: shk3Project Admin

CrossoverMachineTest:
* explain why it is reasonable for the algorithm to fail up to 299 times?
 (and add comments to the source accordingly...)
Basically, it is reasonable because the old one failed 10000 times out of
10000 times. 300 seems like an improvment for me.

* since this is new functionality, please add full unit testing
This should be there
* run the PMD testing on structgen and test-structgen and please fix
the fails for CrossoverMachine and PartialFilledStructureMerger and
tests (~7 fails in your new code in structgen)
The only pmd messages I get relate to complexity etc. There is a lot of
code with such messages and considering that the old code did not work at
all, I don't feel like bothering about this right now.


Attached Files ( 5 )

Filename Description Download
0001-added-dependency.patch Download
0001-added-working-implementations-for-PartialFilledStruc.patch Download
0001-tests-for-crossover-machine-and-PartialFilledStructu.patch Download
0002-tests-for-crossover-machine-and-PartialFilledStructu.patch Download
0002-added-working-implementations-for-PartialFilledStruc.patch Download

Changes ( 6 )

Field Old Value Date By
artifact_group_id None 2009-08-18 11:21 egonw
File Added 339588: 0002-added-working-implementations-for-PartialFilledStruc.patch 2009-08-18 11:05 shk3
File Added 339587: 0002-tests-for-crossover-machine-and-PartialFilledStructu.patch 2009-08-18 11:05 shk3
File Added 339577: 0001-tests-for-crossover-machine-and-PartialFilledStructu.patch 2009-08-18 09:05 shk3
File Added 339576: 0001-added-working-implementations-for-PartialFilledStruc.patch 2009-08-18 09:05 shk3
File Added 339575: 0001-added-dependency.patch 2009-08-18 09:05 shk3