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

BroadcastRegionJoin fails with unmapped reads #821

Closed
arahuja opened this issue Sep 16, 2015 · 4 comments
Closed

BroadcastRegionJoin fails with unmapped reads #821

arahuja opened this issue Sep 16, 2015 · 4 comments

Comments

@arahuja
Copy link
Contributor

@arahuja arahuja commented Sep 16, 2015

This is obvious in hindsight as AlignmentRecord -> ReferenceRegion gives a NullPointerException on unmapped reads.

I opened to the issue to discuss what everyone think the best solution. Things I've thought of are:

  1. Do nothing, assume users will only do this with mapped reads, maybe throw a better warning (as obvious as it is now, I was not clear why this was failing for a while)
  2. Filter unmapped reads for users before they perform the join
  3. Join unmapped regions with some Unmapped region-like placeholder?

2 seems the most reasonable?

@ryan-williams
Copy link
Member

@ryan-williams ryan-williams commented Sep 16, 2015

This is obvious in hindsight as AlignmentRecord -> ReferenceRegion gives a NullPointerException on unmapped reads.

BRJ doesn't do AlignmentRecord -> ReferenceRegion conversions though, does it?

@arahuja
Copy link
Contributor Author

@arahuja arahuja commented Sep 16, 2015

No I guess technically not, it's take things keyed by ReferenceRegion so perhaps AlignmentRecord -> ReferenceRegion should fail with a better error? But I could also imagine BRJ doing this automatically and filtering the unmapped reads.

@laserson
Copy link
Contributor

@laserson laserson commented Sep 16, 2015

I vote for #1. I'd prefer the join impl not do additional operations that may not be obvious.

@fnothaft
Copy link
Member

@fnothaft fnothaft commented Mar 3, 2017

This is resolved by pending PR #1324. Closing in favor of the main ShuffleRegionJoin ticket #1216.

@fnothaft fnothaft closed this Mar 3, 2017
@heuermh heuermh added this to Completed in Release 0.23.0 Mar 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.