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

Enable mapping TELL-Seq and DBS linked reads #51

Merged
merged 8 commits into from
Jun 27, 2023

Conversation

pontushojer
Copy link
Contributor

This PR extends the ema align tool too allow mapping of both TELL-Seq and DBS linked reads.

@pdimens
Copy link
Collaborator

pdimens commented Jun 15, 2023

Thanks for submitting this, awesome! While I'm not one of the ema devs, I was granted collaboration rights to merge PRs. Do you have tests/records confirming this PR hasn't broken anything?

@pontushojer
Copy link
Contributor Author

I have run tests with ema align using subsets of 10x, TELL-Seq and DBS reads to confirm that things look ok. Not Haplotagging though, do you perhaps have some small test set you could send me?

I would probably be good to add some kind of basic testing suite to this repo.

@pdimens
Copy link
Collaborator

pdimens commented Jun 16, 2023

Included is 20 each of forward and reverse haplotag reads for testing purposes:
https://mega.nz/folder/CEp3wSYA#3_LCUTz7S5YP0UazieFbDg

@pontushojer
Copy link
Contributor Author

Thanks for sending the files!

I tried using the files but I am unable to get any output, including when running on the current master branch 1d3b65d! Maybe I am not using the command correctly for haplotag reads. This is the command that I am running:

./ema align -p haplotag -1 <(pigz -cd haplotag.F.fq.gz) -2 <(pigz -cd haplotag.F.fq.gz) -r ref.fasta

This is the output:

stdout

@HD	VN:1.3	SO:unsorted
@SQ	SN:chrA	LN:50000
@SQ	SN:chrB	LN:40000
@SQ	SN:chrC	LN:9000
@SQ	SN:chrD	LN:1000
@RG	ID:rg1	SM:sample1
@PG	ID:ema	PN:ema	VN:0.6.2	CL:./ema align -p haplotag -1 /dev/fd/63 -2 /dev/fd/62 -r ref.fasta

stderr

BWA initialization...
[M::bwa_idx_load_from_disk] read 0 ALT contigs
Processing reads...

It only generates the header, no alignments.

On another note, it would be good to have a bit of a larger test set with multiple reads per barcode. The current files contain only one read pair per barcode which would then skip most of ema's remapping functionality. Also it would be good to have a known reference which they should map to. I have generate testdata for a subsection of GRCh38 chr1 which i used for testing the other technologies.

@pdimens
Copy link
Collaborator

pdimens commented Jun 19, 2023

Sorry about that, I used a minimal set for simplicity. I'll aim for a bigger file and send those links

@pdimens
Copy link
Collaborator

pdimens commented Jun 20, 2023

Try the files now. I replaced them with subsets featuring 1.6M reads apiece.

@pontushojer
Copy link
Contributor Author

@pdimens I have now run the tests using the haplotag reads you provided. I had to make one change but now the output from this PR matches master for haplotag reads.

@pdimens
Copy link
Collaborator

pdimens commented Jun 23, 2023

I saw the new issues you posted. Unfortunately, I'm both inexperienced with C++ and not one of the ema devs. The points you make are quite serious, but I am unable to modify ema to overcome these issues. I was given collaboration rights to merge PRs, but you should try pinging @arshajii

@pontushojer
Copy link
Contributor Author

I saw the new issues you posted. Unfortunately, I'm both inexperienced with C++ and not one of the ema devs. The points you make are quite serious, but I am unable to modify ema to overcome these issues. I was given collaboration rights to merge PRs, but you should try pinging @arshajii

As ema devs, I would guess they already get notifications on this. I am also too inexperienced in C/C++ to make any attempt to fix this.

This PR is however unrelated to the issues I raised. On my end it is ready to merge. What do you think?

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.

2 participants