czakarian / deduper-czakarian Goto Github PK

-I've never used PriorityQueue() before, cool trick to avoid storing everything in memory.

-Minor thing: on line 46, you nest two if statements, but you can avoid this by adding an and statement encompassing both if statements. I did something like this in the RBH assignment.

-This pseudocode is very detailed, it feels like you've already done most of the assignment already! Hard to find issues here, but one area of improvement could be the explanation and variable names aren't entirely clear. Overall excellent job; you understand the assignment, the algorithm does what it's supposed to do, and the functions are all well-documented and their use/importance is obvious.

General Notes

• On your "general strategy section", I think that you may need to increase the number depending on the sequencing run. It seems like, if the position could be upstream or downstream, using 300 would be good for read lengths of less than 150 bp. You may need to use 600 to cover read lengths of up to 300 (which I believe is the longest Illumina makes kits for at the moment). But, you mention that it can be adjusted to whatever value, so that's good. Your idea to use PriorityQueue() is unique! I've never heard of it before but I looked it up and it sounds cool.

Does the proposed algorithm make sense to you? Can you follow the logic?

Yes, everything is laid out real nicely. The section on "SAM file columns of interest" was particularly helpful.

Does the algorithm do everything it's supposed to do? (see part 1)

Yes, looks solid. I'm a little concerned that a 300-item dictionary being searched for every read that goes through may take a lot of time and memory. But I have no way of estimating how much it would take, so I'll be interested to hear if this approach works out.

Are proposed functions reasonable? Are they "standalone" pieces of code?

Yes, they look awesome! I especially liked the one "get_position" where it gathers all the necessary info to determine duplicates.