Skip to content

[SYSTEMDS-3823] Compression test case for bultin kmeans#2194

Closed
e-strauss wants to merge 1 commit intoapache:mainfrom
e-strauss:decompress_left_indexing
Closed

[SYSTEMDS-3823] Compression test case for bultin kmeans#2194
e-strauss wants to merge 1 commit intoapache:mainfrom
e-strauss:decompress_left_indexing

Conversation

@e-strauss
Copy link
Contributor

This fix adds a new test case for the builtin kmeans with compression enabled. It also adds a missing decompression handling for indexed MatrixBlock copy, which previously broke the compressed kmeans test case.

Added missing decompression handling for indexed MatrixBlock copy
@codecov
Copy link

codecov bot commented Jan 29, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.30%. Comparing base (6c4bffd) to head (d4c140d).
Report is 4 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #2194      +/-   ##
============================================
+ Coverage     71.86%   72.30%   +0.43%     
- Complexity    44707    44997     +290     
============================================
  Files          1450     1452       +2     
  Lines        169272   169309      +37     
  Branches      32996    33037      +41     
============================================
+ Hits         121652   122413     +761     
+ Misses        38290    37587     -703     
+ Partials       9330     9309      -21     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@Baunsgaard Baunsgaard left a comment

Choose a reason for hiding this comment

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

LGTM, just two lines to change.

*/
public void copy(int rl, int ru, int cl, int cu, MatrixBlock src, boolean awareDestNZ ) {
if (src instanceof CompressedMatrixBlock){
src = ((CompressedMatrixBlock) src).decompress();
Copy link
Contributor

Choose a reason for hiding this comment

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

add a string to the decompress command to indicate what is causing the decompression.

.getCPHeavyHitterCount("compress") : Statistics.getCPHeavyHitterCount("sp_compress");

Assert.assertEquals("Assert that the compression counts expeted matches actual: " + compressionCount + " vs "
System.out.println(actualCompressionCount);
Copy link
Contributor

Choose a reason for hiding this comment

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

remove System out.

@e-strauss e-strauss closed this in 615cd9a Jan 29, 2025
@e-strauss e-strauss deleted the decompress_left_indexing branch January 29, 2025 12:47
@Baunsgaard
Copy link
Contributor

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants