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

fold halfWindowSize into args plumbing #597

Merged
merged 3 commits into from
Oct 1, 2016

Conversation

ryan-williams
Copy link
Member

No description provided.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.09%) to 78.757% when pulling b855ea5 on ryan-williams:hws into 738996d on hammerlab:master.

Copy link
Contributor

@arahuja arahuja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine to me, I don't necessarily see any advantages/disadvantages here - any background on that?

@ryan-williams
Copy link
Member Author

ryan-williams commented Oct 1, 2016

Sure, sorry I didn't provide more context.

I'm diving back into some loci-partitioning stuff to explore sending only the necessary reference material to each partition, rather than Broadcasting it per-contig.

The halfWindowSize treatment through the loci-partitioning logic felt like it was not encapsulated correctly:

  • one of the three loci-partitioners use it while the other two don't,
  • all callers of loci-partitioning code except for GermlineAssemblyCaller set it to 0 without even offering a flag to configure it,
  • yet it was being plumbed through as a top-level parameter in the LociPartitioner and PartitionedRegions APIs, and hard-coded to 0 in callers that don't care about it.

The API I'm trying to support for loci-partitioners is:

  • everything specific to each partitioner gets passed to that partitioner as a ctor arg, and is hidden from the general LociPartitioner interface.
  • things that are common to all partitioners can be part of the shared interface.

Assembly callers want to map their --assembly-window-range parameter to the half-window-size, so I've done this by having CappedRegionsPartitionerArgs mix-in a halfWindowSize method that AssemblyArgs overrides and maps that flag to.

In a forthcoming PR I add a thin "partition-loci" top-level guac command that wants to do the same thing but with a different flag, so mixing-in and overriding HalfWindowConfig is a way to let multiple commands plumb half-windows through the loci-partitioning logic while allowing them to configure that value through CLI flags (or other logic) of their choice.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 10c595b on ryan-williams:hws into * on hammerlab:master*.

@ryan-williams ryan-williams merged commit be60747 into hammerlab:master Oct 1, 2016
@ryan-williams ryan-williams deleted the hws branch October 1, 2016 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants