Skip to content
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

Refactor Hash Size Warning Logic into Common Module #21

Closed
emcd opened this issue Feb 13, 2013 · 4 comments
Closed

Refactor Hash Size Warning Logic into Common Module #21

emcd opened this issue Feb 13, 2013 · 4 comments

Comments

@emcd
Copy link
Contributor

emcd commented Feb 13, 2013

While manually breaking up lines which were too long, I encountered the same chunk of code copied and pasted many times over and over again in various scripts in scripts/ and sandbox/. We definitely want to place this in a common module for maintainability and reusability reasons.

@susinmotion
Copy link
Contributor

working on this.

@susinmotion
Copy link
Contributor

The threshold for what collision rate is too high varies in different parts of the code.
-Would you like to keep the thresholds as written, even though they are not consistent?
-Would you like to add this check to the places where collision rate is calculated but not checked?

For now, I will pass the existing thresholds as an argument to the check function, and will use .2 as a default threshold where none is listed.

Here are the places the duplicate logic is found (or should be found?), and their thresholds:
Collect variants -- .2 (with a comment saying don't change)
Collect Reads -- .2 (with a comment saying don't change)
Saturate by med. -- .8 (cites Zhang et al)
do partition -- .15 (with a comment saying the actual max is .18, don't change)
filter abund. single-- no check
load graph -- .15 (with a comment saying the actual max is .18, don't change)
load into c. -- .2
norm by med. -- .8 (cites zhang et al)
trim low abund. -- .8 (cites zhang et al)

@susinmotion
Copy link
Contributor

Also!
Some places had the opportunity to force listed and others didn't. Is there a pattern to where where you'd like to allow for a force? Right now, I'm fixing as if you want to allow for a force at any time.

mr-c added a commit that referenced this issue Apr 14, 2015

Verified

This commit was signed with the committer’s verified signature. The key has expired.
sei-vsarvepalli Vijay Sarvepalli
Fix issue #21:  Refactor Hash Size Warning Logic into Common Module
@mr-c
Copy link
Contributor

mr-c commented Apr 14, 2015

Fixed by #944

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants