Skip to content

Claude/fix twin matrix sizes bn n ve#120

Merged
smasongarrison merged 5 commits intomztwinsfrom
claude/fix-twin-matrix-sizes-bnNVe
Feb 13, 2026
Merged

Claude/fix twin matrix sizes bn n ve#120
smasongarrison merged 5 commits intomztwinsfrom
claude/fix-twin-matrix-sizes-bnNVe

Conversation

@smasongarrison
Copy link
Member

Pull Request Template

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes # (reference issue number here, if any)

Type of change

Please delete options that are not relevant (including this line).

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Test A
  • Test B

Test Configuration:

  • Operating System (Mac OS, Windows 10, etc.):
  • Text Editor (Atom, Eclipse, Vim, etc.):
  • Network (local, VPN, etc.):

Checklist:

Requirements Met:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the project documentation
  • My changes generate no new warnings or errors
  • I have checked my code and corrected any misspellings

Recommendations Met:

While these are not absolutely required for a contribution via pull request, we
do encourage these items are completed.

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass with my changes

claude and others added 5 commits February 13, 2026 21:13
Two issues caused the sparse matrices from the two mz_method paths to
differ structurally:

1. The merging method iterated over mz_pairs (row indices) but treated
   the values as IDs. Changed to iterate over mz_id_pairs which holds
   the actual pedigree IDs. Currently masked because test data has
   sequential IDs matching row positions, but would break with
   non-sequential IDs.

2. The different computational paths (column-merge-before-tcrossprod vs
   pedigree-modify-then-post-copy) leave different explicit zeros in the
   sparse matrix. Added Matrix::drop0() before returning to normalize
   the sparse representation so both methods produce structurally
   identical results.

https://claude.ai/code/session_01PXbgV7bdA9sg7St6xBbg3T
The merging method's post-copy (r[idx2,] <- r[idx1,]) causes Matrix
to coerce from dsCMatrix (symmetric, one triangle) to dgCMatrix
(general, both triangles), doubling @i/@x slot lengths. Add
forceSymmetric() after the post-copy to restore symmetric storage.

Also guard the merging pedigree modification with a component check
(additive only) to match the restoration block, and move the verbose
message inside the merging if-block so it only prints when merging
actually occurs.

https://claude.ai/code/session_01PXbgV7bdA9sg7St6xBbg3T
sum(A, B) sums all elements of both matrices, which can never be 0
for non-negative relatedness values. Use sum(A - B) to check that the
element-wise differences are zero.

https://claude.ai/code/session_01PXbgV7bdA9sg7St6xBbg3T
@smasongarrison smasongarrison merged commit f33aed8 into mztwins Feb 13, 2026
1 of 2 checks passed
@codecov
Copy link

codecov bot commented Feb 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (mztwins@c2138ae). Learn more about missing BASE report.

Additional details and impacted files
@@            Coverage Diff             @@
##             mztwins     #120   +/-   ##
==========================================
  Coverage           ?   84.15%           
==========================================
  Files              ?       28           
  Lines              ?     4531           
  Branches           ?        0           
==========================================
  Hits               ?     3813           
  Misses             ?      718           
  Partials           ?        0           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@smasongarrison smasongarrison deleted the claude/fix-twin-matrix-sizes-bnNVe branch February 14, 2026 01:27
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