-
Notifications
You must be signed in to change notification settings - Fork 294
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add --max-memory-usage argument, and associated khmer_args refactoring. #1050
Conversation
Fixes #980 in passing. |
This is one for @mr-c to gaze upon. So far, I am happy with the direction it is taking. |
…args2 Conflicts: ChangeLog scripts/normalize-by-median.py
I think I will wait on @mr-c to take a look-see before continuing with fixing up the documentation etc. etc. |
…args2 Conflicts: ChangeLog khmer/__init__.py khmer/khmer_args.py oxli/build_graph.py sandbox/collect-reads.py sandbox/saturate-by-median.py scripts/abundance-dist-single.py scripts/do-partition.py scripts/filter-abund-single.py scripts/load-into-counting.py scripts/normalize-by-median.py scripts/trim-low-abund.py tests/test_script_arguments.py
counting hash/hashbits creation with new khmer_args create* functions. | ||
* oxli/build_graph.py,sandbox/{sweep-files.py,sweep-reads.py}, | ||
scripts/{count-overlap.py,do-partition.py,},khmer/khmer_args.py: | ||
changed hashtype over to 'nodegraph' and 'bitgraph' in call to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/bitgraph/countgraph/
…args2 Conflicts: ChangeLog
ARGH this one is going down the rabbit hole... fine, I'll just do it all, then. |
* make CountingHash a Python wrapper around _CountingHash; * replace new_counting_hash with CountingHash through codebase; * replace new_hashbits with Hashbits throughout codebase; Still some tests failing, though.
Well, all of these inter-related issues provide a lot of value and are a long time coming :-) |
Conflicts: khmer/__init__.py tests/test_counting_hash.py tests/test_hashbits_obj.py
test coverage issues fixed.
|
test this please, jenkins. |
retest this, please |
Ready for review and merge @mr-c. |
Note: we will probably need to redo things in doc/user/scripts.rst and choosing-hash-sizes a bit once #1106 lands; please evaluate current doc updates in this light :) |
New parameter for tablesize/number of table parameters. | ||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
||
There is now a `-M/--max-memory-usage` parameter that sets the number |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should use the style
:option:`-M`/:option:`--max-memory-usage`
throughout this document.
Great! I'm going to fix the two small nits I found and merge for you. |
fixed the doc one On Fri, Jun 26, 2015 at 05:20:09PM -0700, Michael R. Crusoe wrote:
|
sigh Thanks :-P On Fri, Jun 26, 2015 at 5:21 PM C. Titus Brown [email protected]
|
Add --max-memory-usage argument, and associated khmer_args refactoring.
lol On Fri, Jun 26, 2015 at 05:27:33PM -0700, Michael R. Crusoe wrote:
|
great, thanks! cc @bocajnotnef
|
This pull request should make little practical difference in behavior, but does several major refactors:
-M/--max-memory-usage
which is a single parameter to automatically setgraph sizes;
below the specified number are chosen. This necessitated many small changes to tests that
relied on specific behavior and numbers.
Python-land to subclass the CPython class _CountingHash;
new_counting_hash and new_hashbits with CountingHash and Hashbits.
and then removed test_hashbits_obj.
and of course many minor adjustments throughout.
-M/--max-memory-usage
parameter in khmer_args.py (try 2 to address User experience upgrades for khmer #732 (comment); try 1 is max memory parameter specification #744)create_nodegraph/create_countgraph
functions from khmer_args.pycheck_space_for_hashtable
to use 'args' rather than 'min_tablesize'