Menu
▾
▴
[Cdk-devel] [ cdk-Patches-2378524 ] further speedup of FingerPrinter
|
From: SourceForge.net <no...@so...> - 2008-12-04 17:13:04
|
Patches item #2378524, was opened at 2008-12-02 18:30 Message generated for change (Comment added) made by jonalv You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=320024&aid=2378524&group_id=20024 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: None Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: Jonathan Alvarsson (jonalv) Assigned to: Nobody/Anonymous (nobody) Summary: further speedup of FingerPrinter Initial Comment: Caching of expensive calculating which turns out to be made many times. Also moved String declaration out of method which can be called quite a few times... ---------------------------------------------------------------------- >Comment By: Jonathan Alvarsson (jonalv) Date: 2008-12-04 18:13 Message: I have made a branch: (http://cdk.svn.sourceforge.net/viewvc/cdk/cdk/branches/jonalv-fingerprint-1.2.x/) where I have carefully redone everything that was in this patch (+ a lot more...) and run the FingerPrintTest continously so it passes. Statistics for the speed up along this branch can be viewed at: http://spreadsheets.google.com/ccc?key=pdSdCFxXYzmNP313WvRy8dw ---------------------------------------------------------------------- Comment By: Jonathan Alvarsson (jonalv) Date: 2008-12-03 20:29 Message: It was never my intention to change the result. If the result has changed, then what I have done is borken somehow and requires further study. I whish I knew why though... ---------------------------------------------------------------------- Comment By: Egon Willighagen (egonw) Date: 2008-12-03 20:09 Message: Yes, the FPs changing actually is a problem... BTW, I think they might have changed too resulting from the recernt Element NULL update... as one builder3d test has been failing since... not sure what this FP code is doing exactly... but it changes with every SVN revision it seems :( The code changes Jonathan and I looked at only changed the way paths are stored, not how they are generated, but I was already afraid of this... but did not had time to verify myself yet... this is actually one reason why I am so eager on getting Nightly work well, so that it is cheap to set one up for whatever branch... I will check with Jonathan tomorrow what might be causing the paths to change... ---------------------------------------------------------------------- Comment By: Rajarshi Guha (rajarshi) Date: 2008-12-03 19:40 Message: Impresssive speedup! 27s vs 43s to generate fingperprints for 1000 molecules from the cdk-qa project. However My understanding was that the patch was designed to make storage and checking of paths more efficient, rather than changing the paths themselves. The reason I note this is that the fingerprints are different from the original (i.e. current trunk version) as indicated by the unit test. This implies that other stuff (such as 3D builder) will need to be updated. Is this a concern? ---------------------------------------------------------------------- Comment By: Jonathan Alvarsson (jonalv) Date: 2008-12-03 10:36 Message: Oh right. Sorry. egonw had already performed some changes to the Bioclipse version of this. Here follows a patch made towards cdk trunk that contains both egonw's changes and mine. File Added: patch.txt ---------------------------------------------------------------------- Comment By: Rajarshi Guha (rajarshi) Date: 2008-12-03 00:01 Message: Can you provide a patch against CDK Trunk - I can't apply your complete patch (1 chunk fails) but then I can't rebuild the distro since it seems that GraphOnlyFingerprinter was not updated. Also I get the error from Fingerprinter: [javac] /Users/rguha/src/java/cdk/trunk/build/src/standard/org/openscience/cdk/fingerprint/Fingerprinter.java:144: incompatible types [javac] found : java.util.Set<java.lang.String> [javac] required: java.util.Map<java.lang.String,java.lang.String> [javac] Map<String,String> paths = findPathes(container, searchDepth); [javac] ^ [javac] /Users/rguha/src/java/cdk/trunk/build/src/standard/org/openscience/cdk/fingerprint/Fingerprinter.java:220: cannot find symbol [javac] symbol : method put(java.lang.String,java.lang.String) [javac] location: interface java.util.Set<java.lang.String> [javac] for (String s : cleanPath) paths.put(s, s); ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=320024&aid=2378524&group_id=20024 |