Open
Conversation
- Replace boost::random with std::random - Replace boost::math::normal/cdf with std::erf-based implementation - Update Makefile: -std=c++17, remove BOOST - Fix C++11/17 compatibility in buildReadIndex, SingleHit, PairedEndHit
- Remove boost dependencies in bed2vector and bamread files - Replace hash_map and hash_set
- Add Makefile test targets: test-prepare-reference, test-calculate-expression, test-simulate-reads - Add test-all for CI (continue on failure) - Add generate-gold target to regenerate gold standard - Use SARS-CoV-2 reference (sarscov2.fasta, sarscov2.gtf) and reads - Gold includes: reference (.grp, .ti), expression (genes.results, isoforms.results, stat/), simulated (fq, sim.isoforms.results, sim.genes.results) - TEST_THREADS=4, TEST_SEED=0, N=1000 for reproducibility - Add tests/output/ to .gitignore
- Boost-compatible RNG for reproducible results
- Copy all rsem-prepare-reference outputs to gold (grp, ti, chrlist, idx.fa, n2g.idx.fa, seq, transcripts.fa, bt2 index files) - Update test-prepare-reference to diff all reference files
…t-calculate-expression target
…l tests - Set TEST_THREADS=1 so Gibbs/calcCI produce reproducible output - Replace CI sanity checks with exact diff for genes.results, isoforms.results - generate-gold now invokes generate-gold-ci (all gold in one run) - Add expression_ci gold (genes, isoforms, stat)
cndewey
reviewed
Apr 3, 2026
| @echo "==> Testing rsem-simulate-reads" | ||
| rm -rf $(TEST_OUTPUT)/simulated | ||
| mkdir -p $(TEST_OUTPUT)/simulated | ||
| @theta0=$$(awk 'NR==3 {print $$1}' $(TEST_OUTPUT)/expression/$(SAMPLE_NAME).stat/$(SAMPLE_NAME).theta) && \ |
Member
There was a problem hiding this comment.
have this depend on
cndewey
reviewed
Apr 3, 2026
| diff $(TEST_OUTPUT)/expression_ci/$(SAMPLE_NAME_CI).stat/$(SAMPLE_NAME_CI).model $(GOLD)/expression_ci/$(SAMPLE_NAME_CI).stat/$(SAMPLE_NAME_CI).model | ||
| diff $(TEST_OUTPUT)/expression_ci/$(SAMPLE_NAME_CI).stat/$(SAMPLE_NAME_CI).theta $(GOLD)/expression_ci/$(SAMPLE_NAME_CI).stat/$(SAMPLE_NAME_CI).theta | ||
|
|
||
| test-simulate-reads: test-calculate-expression |
Member
There was a problem hiding this comment.
remove dependency on test-calculate-expression. Should just use $(GOLD)
cndewey
reviewed
Apr 3, 2026
| ./rsem-simulate-reads \ | ||
| $(GOLD)/reference/$(REF_NAME) \ | ||
| $(TEST_OUTPUT)/expression/$(SAMPLE_NAME).stat/$(SAMPLE_NAME).model \ | ||
| $(TEST_OUTPUT)/expression/$(SAMPLE_NAME).isoforms.results \ |
cndewey
reviewed
Apr 3, 2026
| @theta0=$$(awk 'NR==3 {print $$1}' $(TEST_OUTPUT)/expression/$(SAMPLE_NAME).stat/$(SAMPLE_NAME).theta) && \ | ||
| ./rsem-simulate-reads \ | ||
| $(GOLD)/reference/$(REF_NAME) \ | ||
| $(TEST_OUTPUT)/expression/$(SAMPLE_NAME).stat/$(SAMPLE_NAME).model \ |
cndewey
reviewed
Apr 3, 2026
| diff $(TEST_OUTPUT)/expression/$(SAMPLE_NAME).stat/$(SAMPLE_NAME).model $(GOLD)/expression/$(SAMPLE_NAME).stat/$(SAMPLE_NAME).model | ||
| diff $(TEST_OUTPUT)/expression/$(SAMPLE_NAME).stat/$(SAMPLE_NAME).theta $(GOLD)/expression/$(SAMPLE_NAME).stat/$(SAMPLE_NAME).theta | ||
|
|
||
| test-calculate-expression-ci: test-prepare-reference |
Member
There was a problem hiding this comment.
remove dependency on test-prepare-reference
cndewey
reviewed
Apr 3, 2026
| diff $(TEST_OUTPUT)/reference/$$bf $$f; \ | ||
| done | ||
|
|
||
| test-calculate-expression: test-prepare-reference |
cndewey
reviewed
Apr 3, 2026
Member
There was a problem hiding this comment.
should these and other related changes be in the remove_boost branch?
cndewey
reviewed
Apr 3, 2026
|
|
||
| # ---- Individual tests -------------------------------------------- | ||
|
|
||
| test-prepare-reference: |
Member
There was a problem hiding this comment.
it would be helpful to have a message printed at the end of each test indicating success
Member
|
@Jeevesh28 - excellent work getting these test cases set up. I have left some specific comments within the files. Some other comments:
|
- Print OK after each test, drop redundant prerequisites - Replace reads.fastq with subset (10k reads) - Regenerate tests/gold expression
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add:
make test-prepare-referencemake test-calculate-expressionmake test-calculate-expression-cimake test-simulate-readsmake testmake test-all