Skip to content

NNZ calcuation fixes (account for delete)#4050

Merged
bkmartinjr merged 9 commits intomainfrom
bkm/nnz-fixes
May 13, 2025
Merged

NNZ calcuation fixes (account for delete)#4050
bkmartinjr merged 9 commits intomainfrom
bkm/nnz-fixes

Conversation

@bkmartinjr
Copy link
Copy Markdown
Member

@bkmartinjr bkmartinjr commented May 12, 2025 •
edited
Loading

Issue and/or context:

The nnz/count C++ method had a number of issues:

  • it ignored delete commits, and therefore reported a higher count than exists for arrays that had deleted cells
  • it would fall back on slow mode for consolidated fragments for arrays with updates

Changes:

  • Add a new private API for counting the number of cells in all fragments as of a given timestamp. This is used by internal partitioner as an approximation of nnz/count.
  • nnz now uses only the COUNT aggregation, which will produce an accurate result for all array conditions

Notes for Reviewer:

@bkmartinjr bkmartinjr marked this pull request as ready for review May 13, 2025 00:26
@bkmartinjr bkmartinjr marked this pull request as draft May 13, 2025 00:28
bkmartinjr added 2 commits May 12, 2025 17:51
@bkmartinjr bkmartinjr marked this pull request as ready for review May 13, 2025 00:59
@codecov
Copy link
Copy Markdown

codecov Bot commented May 13, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 89.49%. Comparing base (4663e08) to head (777c23b).
Report is 3 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #4050      +/-   ##
==========================================
+ Coverage   89.19%   89.49%   +0.30%     
==========================================
  Files          57       57              
  Lines        6902     6903       +1     
==========================================
+ Hits         6156     6178      +22     
+ Misses        746      725      -21
Flag Coverage Δ
python 89.49% <60.00%> (+0.30%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
python_api 89.49% <60.00%> (+0.30%) ⬆️
libtiledbsoma ∅ <ø> (∅)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@bkmartinjr bkmartinjr mentioned this pull request May 13, 2025
Closed
@bkmartinjr bkmartinjr merged commit 44af843 into main May 13, 2025
21 checks passed
@bkmartinjr bkmartinjr deleted the bkm/nnz-fixes branch May 13, 2025 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jp-dark jp-dark jp-dark approved these changes

@XanthosXanthopoulos XanthosXanthopoulos Awaiting requested review from XanthosXanthopoulos

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@bkmartinjr @jp-dark