Skip to content

Fix twin matrix size mismatch between addtwins and merging methods#119

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

Fix twin matrix size mismatch between addtwins and merging methods#119
smasongarrison merged 2 commits intomztwinsfrom
claude/fix-twin-matrix-sizes-bnNVe

Conversation

@smasongarrison
Copy link
Member

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

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 2 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
@smasongarrison smasongarrison merged commit 9d4d507 into mztwins Feb 13, 2026
0 of 2 checks passed
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