Skip to content

Fix MATLAB size_t scalar wrapping on LP64 macOS - #195

Merged
dellaert merged 2 commits into
masterfrom
codex/fix-matlab-size-t
Aug 11, 2026
Merged

Fix MATLAB size_t scalar wrapping on LP64 macOS#195
dellaert merged 2 commits into
masterfrom
codex/fix-matlab-size-t

Conversation

@dellaert

Copy link
Copy Markdown
Member

Summary

  • route size_t and uint64_t MATLAB conversions through alias-aware primary-template dispatch
  • avoid duplicate explicit specializations when the two names denote the same C++ type
  • treat generated uint64_t arguments and results as MATLAB numeric scalars
  • retain the generic runtime errors for unsupported wrap and unwrap types
  • add focused generation coverage for size_t and uint64_t round trips

Root cause

The prior preprocessor guard assumed every LP64 platform aliases size_t to uint64_t. That is true for common Linux definitions but false on 64-bit macOS, where the types have the same width but distinct underlying C++ types. As a result, macOS omitted unwrap<size_t> and generated calls fell through to the generic error-producing implementation.

Using C++ type traits makes the decision from actual type identity instead of platform macros, so macOS, Linux, Windows, and CUDA configurations cannot produce either a missing conversion or duplicate explicit specializations.

Validation

  • conda run -n py312 python -m pytest tests/test_matlab_wrapper.py -q — 12 passed
  • conda run -n py312 python -m pytest tests -q — 111 passed
  • git diff --check
  • synced into a local GTSAM feature branch with ./update_wrap.sh codex/fix-matlab-size-t
  • built and installed the GTSAM MATLAB R2025b wrapper on arm64 macOS
  • verified gtsam.KalmanFilter(2) constructs successfully
  • verified gtsam.symbol('x', 1) and gtsam.symbolIndex round-trip successfully
  • the canonical MATLAB runner passed all size/key-related tests without the former unwrap<size_t> error; its final optional Boost serialization test encountered a separate Homebrew Boost/std::locale allocator abort

@dellaert
dellaert marked this pull request as ready for review August 11, 2026 16:14
@dellaert
dellaert requested a lite review from Copilot August 11, 2026 16:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@dellaert
dellaert requested a review from ProfFan August 11, 2026 16:15

@ProfFan ProfFan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@dellaert
dellaert merged commit 6dce7be into master Aug 11, 2026
8 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.

3 participants