Add RANS Nx16 codec (Update CRAM Codecs to CRAM 3.1) by yash-puligundla · Pull Request #1618 · samtools/htsjdk

yash-puligundla · 2022-08-26T15:44:08Z

Description

This PR is part of an effort to upgrade CRAM to v3.1. It adds the RANS Nx16 implementation and adds changes to the existing RANS implementation to accommodate RANS Nx16.

List of Changes:

Add RANS Nx16 Codec
Add interfaces - RANSEncode, RANSDecode and RANSParams
Restructured existing RANS code
- Rename the existing RANS codec to RANS4x8
- Change the structure of the ArithmeticDecoder
- Move the code from E04, E14, D04, and D14 into the classes where it is used
Add CRAMCodecCorpusTest to test RANS 4x8 and RANS Nx16 Codecs using test files from htscodecs
Add more test cases to RANSTest to test RANS 4x8 and RANS Nx16 Codecs

Test fails:
CRAMCodecCorpusTest fails as they require the test files from htscodecs repo.

cmnbroad

Checkpointing what I have so far, which only covers test code.

src/test/java/htsjdk/samtools/cram/CRAMCodecCorpus.java

src/test/java/htsjdk/samtools/cram/CRAMCodecCorpusTest.java

src/test/java/htsjdk/samtools/cram/CRAMCodecCorpus.java

src/test/java/htsjdk/samtools/cram/CRAMCodecCorpusTest.java

src/test/java/htsjdk/samtools/cram/compression/rans/RansTest.java

src/test/java/htsjdk/samtools/cram/CRAMCodecCorpusTest.java

cmnbroad

A few more minor comments.

src/test/java/htsjdk/samtools/cram/RANSInteropTest.java

src/test/java/htsjdk/samtools/cram/RANSInteropTestUtils.java

codecov-commenter · 2022-09-09T19:03:13Z

Codecov Report

Merging #1618 (c63a253) into master (347c0ac) will increase coverage by 0.423%.
Report is 40 commits behind head on master.
The diff coverage is 95.548%.

❗ Current head c63a253 differs from pull request most recent head 1979264. Consider uploading reports for the commit 1979264 to get more accurate results

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@               Coverage Diff               @@
##              master     #1618       +/-   ##
===============================================
+ Coverage     69.856%   70.279%   +0.423%     
- Complexity      9695      9907      +212     
===============================================
  Files            703       705        +2     
  Lines          37772     38427      +655     
  Branches        6139      6275      +136     
===============================================
+ Hits           26386     27006      +620     
- Misses          8929      8945       +16     
- Partials        2457      2476       +19

Files	Coverage Δ
.../samtools/cram/compression/ExternalCompressor.java	`74.074% <100.000%> (ø)`
...tools/cram/compression/RANSExternalCompressor.java	`76.000% <100.000%> (+5.167%)`	⬆️
...tools/cram/compression/rans/ArithmeticDecoder.java	`100.000% <100.000%> (ø)`
...jdk/samtools/cram/compression/rans/RANSDecode.java	`100.000% <100.000%> (ø)`
...ools/cram/compression/rans/RANSDecodingSymbol.java	`100.000% <100.000%> (ø)`
...jdk/samtools/cram/compression/rans/RANSParams.java	`100.000% <100.000%> (ø)`
...s/cram/compression/rans/rans4x8/RANS4x8Params.java	`100.000% <100.000%> (ø)`
...cram/compression/rans/ransnx16/RANSNx16Params.java	`100.000% <100.000%> (ø)`
...s/cram/structure/CompressionHeaderEncodingMap.java	`85.621% <100.000%> (ø)`
...tsjdk/samtools/cram/structure/CompressorCache.java	`92.000% <100.000%> (+0.696%)`	⬆️
... and 8 more

... and 25 files with indirect coverage changes

yash-puligundla · 2022-09-13T20:51:06Z

Hi @cmnbroad I have addressed all the feedback so far. Please let me know if you notice anything. Thanks!

cmnbroad

Another checkpoint of what I have so far.

src/main/java/htsjdk/samtools/cram/compression/rans/ransnx16/RANSNx16Params.java

src/test/java/htsjdk/samtools/cram/RANSInteropTest.java

src/test/java/htsjdk/samtools/cram/compression/rans/RansOrder1DemoTest.java

src/test/java/htsjdk/samtools/cram/RANSInteropTest.java

src/main/java/htsjdk/samtools/cram/compression/rans/RANSEncodingSymbol.java

src/main/java/htsjdk/samtools/cram/compression/rans/rans4x8/RANS4x8Params.java

src/test/java/htsjdk/samtools/cram/compression/rans/RansTest.java

cmnbroad

Not even close to done, but checkpointing what I have - will resume when I get back from vacation.

src/test/java/htsjdk/samtools/cram/RANSInteropTest.java

src/main/java/htsjdk/samtools/cram/compression/rans/ransnx16/RANSNx16Decode.java

cmnbroad

Still a little bit more to go, but almost done.

src/test/java/htsjdk/samtools/cram/RANSInteropTest.java

src/test/java/htsjdk/samtools/cram/compression/rans/RansTest.java

src/main/java/htsjdk/samtools/cram/compression/rans/RANSEncode.java

src/main/java/htsjdk/samtools/cram/compression/rans/RANSEncodingSymbol.java

src/main/java/htsjdk/samtools/cram/compression/rans/ransnx16/RANSNx16Decode.java

cmnbroad

In this round I mostly focused on the decoders - many of my comments apply to the encoders as well, but I didn't duplicate them there since there are quite a few. I'm stopping for now and will need to do another round once these issues are addressed.

src/main/java/htsjdk/samtools/cram/compression/rans/ArithmeticDecoder.java

src/main/java/htsjdk/samtools/cram/structure/CompressorCache.java

src/main/java/htsjdk/samtools/cram/compression/rans/ransnx16/RANSNx16Encode.java

src/main/java/htsjdk/samtools/cram/compression/rans/ransnx16/RANSNx16Decode.java

yash-puligundla · 2024-01-02T16:31:23Z

@cmnbroad Thank you for your comments. I have addressed all of them so far. Ready for the next round of review.

…dded in the streams).

…ests or ../htscodecs/tests depending on the env

…ider

…ipe Flag in RANS Nx16 encoder

…htscodecs repo.

cmnbroad · 2025-02-11T16:33:09Z

Closing in favor of #1714.

cmnbroad self-assigned this Aug 29, 2022

cmnbroad reviewed Aug 30, 2022

View reviewed changes

cmnbroad requested changes Sep 6, 2022

View reviewed changes

yash-puligundla requested a review from cmnbroad September 13, 2022 20:49

cmnbroad requested changes Oct 25, 2022

View reviewed changes

yash-puligundla requested a review from cmnbroad October 31, 2022 21:05

yash-puligundla force-pushed the yp_cram_3_1_codecs branch from a2d9dad to 7e06b5d Compare January 12, 2023 20:53

yash-puligundla mentioned this pull request Jan 12, 2023

Add Range codec (Update CRAM Codecs to CRAM 3.1) #1644

Closed

This was referenced Mar 21, 2023

Add NameTokenization Decoder yash-puligundla/htsjdk#4

Closed

Add Name Tokenization Codec (Update CRAM Codecs to CRAM 3.1) #1663

Closed

cmnbroad requested changes Oct 11, 2023

View reviewed changes

yash-puligundla force-pushed the yp_cram_3_1_codecs branch from aeec2ad to f7e6c57 Compare October 20, 2023 18:36

cmnbroad requested changes Nov 7, 2023

View reviewed changes

cmnbroad requested changes Nov 20, 2023

View reviewed changes

yash-puligundla force-pushed the yp_cram_3_1_codecs branch from 61b0eca to e6b06a5 Compare December 6, 2023 22:36

yash-puligundla mentioned this pull request Feb 7, 2024

Add FQZComp Codec (Update CRAM Codecs to CRAM 3.1) #1704

Closed

cmnbroad force-pushed the yp_cram_3_1_codecs branch from dc12137 to cccda09 Compare February 27, 2024 17:30

yash-puligundla and others added 10 commits March 5, 2024 14:31

adding comments to Frequencies.java

e493a5a

separate encode and decode classes

9d9e681

Add Frequency methods to encode and decode classes

a84de97

clean up rans tests and add separate packages for rans 4x8 and nx16

17073e1

filter out extra column from q40+dir file

8d88da0

rans nx16 order 1 freq tables + refactor

3ec829a

clean up

835bdf6

Update RAN test method names.

44548c3

Remove unncessary params arg from uncompress methods (params are embe…

6f3e9d5

…dded in the streams).

Remove unnecessary RANSNx16Params state.

9548daf

yash-puligundla and others added 24 commits March 5, 2024 14:32

Use the Interop Test files from samtools-1.14/htslib-1.14/htscodecs/t…

c514bee

…ests or ../htscodecs/tests depending on the env

Replace hex literals with bit flag masks in RANSInteropTest Data Prov…

3ee96c5

…ider

Addressing the feedback so far

4129cb3

rename methods that return boolean to start with 'is' instead of 'get'

0ffc6ba

debug

947e8e4

Addressing the feedback from 10/25/22

a04ede6

undo inadvertent deletion of RANSInterop roundtrip test logic

266ec6d

debug - add decodePack and decodeRLE on top of CAT flag

1ecb5c0

rewind outBuffer before it is returned

06a89a8

remove duplicate outBuffer creation

04c813f

Addressing the feedback from oct 11, 2023 except implementing the Str…

dff9d51

…ipe Flag in RANS Nx16 encoder

Move common methods to CRAMInteropTestUtils class

3b25380

Addressing the feedback from Nov 7 and Nov 20 - part 1

015491b

Addressing the feedback from Nov 7 and Nov 20 - part 2

4341721

Addressing the feedback from Nov 7 and Nov 20 - part 3

c7a06a9

Addressing the feedback from Nov 7 and Nov 20 - part 4

bc7cced

Addressing the feedback from Nov 7 and Nov 20 - part 5

c0ff577

Addressing the feedback from Nov 7 and Nov 20 - part 6

7d72393

Addressing the feedback from Nov 7 and Nov 20 - part 6

e898080

Move common code to CompressionUtils

250f901

Minor final review comments.

6449de8

Use copies of the CRAM 3.1 interop test streams that originate inthe …

c411ada

…htscodecs repo.

Update to use the locally checked in CRAM 3.1 interop test data.

fe0e908

A little naming cleanup.

8ed004f

cmnbroad force-pushed the yp_cram_3_1_codecs branch from 1979264 to 8ed004f Compare March 5, 2024 20:19

Fix issue introduced by conflict resolution of a rebase.

f1f9c20

cmnbroad mentioned this pull request Sep 4, 2024

Implementation of CRAM 3.1 codecs. #1714

Merged

lbergelson added the cram label Oct 1, 2024

cmnbroad closed this Feb 11, 2025

Conversation

yash-puligundla commented Aug 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

cmnbroad left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cmnbroad left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Sep 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

yash-puligundla commented Sep 13, 2022

Uh oh!

cmnbroad left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cmnbroad left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cmnbroad left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cmnbroad left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

yash-puligundla commented Aug 26, 2022 •

edited

Loading

codecov-commenter commented Sep 9, 2022 •

edited

Loading