Fix Eigen::Ref<MatrixXd> Jacobian output args in MATLAB wrapper

gtwrap/matlab_wrapper/mixins.py

@@ -22,6 +22,8 @@ class CheckMixin:
     ignore_namespace: Tuple = ('Matrix', 'Vector', 'Point2', 'Point3')
     # Matrix-like view types that can alias MATLAB double matrix storage.
     matrix_view_types: Tuple = ('ConstMatrixView', )
+    # Eigen Ref types used as Jacobian output arguments (not inputs).
+    eigen_ref_types: Tuple = ('MatrixXd', )
     # Methods that should be ignored
     ignore_methods: Tuple = ('pickle', )
     # Methods that should not be wrapped directly
@@ -74,6 +76,20 @@ def is_matrix_view(self, arg_type: parser.Type):
         """Check if `arg_type` should be unwrapped as a matrix view."""
         return arg_type.typename.name in self.matrix_view_types
 
+    def is_eigen_ref(self, arg_type) -> bool:
+        """Check if `arg_type` is an Eigen Ref output argument (e.g. Ref<MatrixXd>).
+
+        These are Jacobian output arguments in C++ that should be treated as
+        extra return values in MATLAB rather than input arguments.
+        Eigen::Ref<Eigen::MatrixXd> parses as a TemplatedType with
+        typename.name == 'Ref' and a MatrixXd template parameter.
+        """
+        return (arg_type.typename.name == 'Ref'
+                and hasattr(arg_type, 'template_params')
+                and len(arg_type.template_params) == 1
+                and arg_type.template_params[0].typename.name
+                in self.eigen_ref_types)
+
     def is_class_enum(self, arg_type: parser.Type, class_: parser.Class):
         """Check if arg_type is an enum in the class `class_`."""
         if class_:

gtwrap/matlab_wrapper/wrapper.py

@@ -301,11 +301,18 @@ def _wrap_method_check_statement(self, args: parser.ArgumentList):
         """
         arg_id = 1
 
-        param_count = len(args)
+        # Eigen Ref args are output arguments, not inputs — exclude from count.
+        eigen_ref_count = sum(1 for arg in args.list()
+                              if self.is_eigen_ref(arg.ctype))
+        param_count = len(args) - eigen_ref_count
         check_statement = 'if length(varargin) == {param_count}'.format(
             param_count=param_count)
 
         for _, arg in enumerate(args.list()):
+            # Eigen Ref args are outputs — skip isa() check entirely.
+            if self.is_eigen_ref(arg.ctype):
+                continue
+
             name = arg.ctype.typename.name
 
             if name in self.not_check_type:
@@ -340,6 +347,11 @@ def _wrap_method_check_statement(self, args: parser.ArgumentList):
 
             arg_id += 1
 
+        # If there are Ref output args, require nargout to match.
+        if eigen_ref_count > 0:
+            check_statement += ' && nargout == {n}'.format(
+                n=eigen_ref_count + 1)
+
         check_statement = check_statement \
             if check_statement == '' \
             else check_statement + '\n'
@@ -361,6 +373,12 @@ def _unwrap_argument(self, arg, arg_id=0, instantiated_class=None):
             unwrap = 'unwrapMatrixView< {ctype} >(in[{id}]);'.format(
                 ctype=arg_type, id=arg_id)
 
+        elif self.is_eigen_ref(arg.ctype):
+            # Ref<MatrixXd> is a Jacobian output arg — allocate locally,
+            # do not consume from in[]. Returned via out[] after the call.
+            arg_type = "Eigen::MatrixXd"
+            unwrap = 'Eigen::MatrixXd();'
+
         elif self.is_ref(arg.ctype):  # and not constructor:
             arg_type = "{ctype}&".format(ctype=ctype_sep)
             unwrap = '*unwrap_shared_ptr< {ctype} >(in[{id}], "ptr_{ctype_camel}");'.format(
@@ -410,7 +428,9 @@ def _wrapper_unwrap_arguments(self,
                     '''.format(arg_type=arg_type, name=arg.name,
                                unwrap=unwrap)),
                                          prefix='  ')
-            arg_id += 1
+            # Eigen Ref args don't consume an in[] slot — don't advance arg_id.
+            if not self.is_eigen_ref(arg.ctype):
+                arg_id += 1
 
         params = ''
         explicit_arg_names = [arg.name for arg in args.list()]
@@ -424,7 +444,8 @@ def _wrapper_unwrap_arguments(self,
                 params += arg.default
                 continue
 
-            if not self.is_ref(arg.ctype) and (self.is_shared_ptr(arg.ctype) or \
+            if not self.is_eigen_ref(arg.ctype) and \
+                    not self.is_ref(arg.ctype) and (self.is_shared_ptr(arg.ctype) or \
                 self.is_ptr(arg.ctype) or self.can_be_pointer(arg.ctype)) and \
                     not self.is_enum(arg.ctype, instantiated_class) and \
                     arg.ctype.typename.name not in self.ignore_namespace:
@@ -1366,6 +1387,9 @@ def wrap_collector_function_return(self, method, instantiated_class=None):
 
         params = self._wrapper_unwrap_arguments(
             method.args, arg_id=1, instantiated_class=instantiated_class)[0]
+        # Capture Ref output args before method may be reassigned to a string below.
+        eigen_ref_args = [arg for arg in method.args.backup.list()
+                          if self.is_eigen_ref(arg.ctype)]
 
         return_1 = method.return_type.type1
         return_count = self._return_count(method.return_type)
@@ -1403,6 +1427,11 @@ def wrap_collector_function_return(self, method, instantiated_class=None):
             if return_count == 1:
                 expanded += self._collector_return(
                     obj, return_1, instantiated_class=instantiated_class)
+
+                # Write any Eigen Ref (Jacobian) output args to out[1], out[2], ...
+                for i, ref_arg in enumerate(eigen_ref_args):
+                    expanded += '\n  out[{i}] = wrap< Eigen::MatrixXd >({name});'.format(
+                        i=i + 1, name=ref_arg.name)
 
             elif return_count == 2:
                 return_2 = method.return_type.type2
@@ -1570,7 +1599,8 @@ def generate_collector_function(self, func_id):
                     min1='-1' if is_method else '',
                     shared_obj=shared_obj,
                     method_name=method_name,
-                    num_args=len(extra.args.list()),
+                    num_args=len([a for a in extra.args.list()
+                                  if not self.is_eigen_ref(a.ctype)]),
                     body_args=body_args,
                     return_body=return_body)
 

tests/fixtures/eigen_ref.i

@@ -0,0 +1,30 @@
+namespace gtsam {
+
+#include <gtsam/geometry/Pose3.h>
+
+class Pose3 {
+  Pose3();
+
+  // Case 1: two Ref args, primitive non-Ref input
+  gtsam::Point3 transformFrom(const gtsam::Point3& point) const;
+  gtsam::Point3 transformFrom(const gtsam::Point3& point,
+                              Eigen::Ref<Eigen::MatrixXd> Hself,
+                              Eigen::Ref<Eigen::MatrixXd> Hpoint) const;
+
+  // Case 2: single Ref arg, no other inputs
+  gtsam::Pose3 inverse() const;
+  gtsam::Pose3 inverse(Eigen::Ref<Eigen::MatrixXd> H) const;
+
+  // Case 3: two Ref args, class-type non-Ref input
+  gtsam::Pose3 between(const gtsam::Pose3& pose) const;
+  gtsam::Pose3 between(const gtsam::Pose3& pose,
+                       Eigen::Ref<Eigen::MatrixXd> H1,
+                       Eigen::Ref<Eigen::MatrixXd> H2) const;
+
+  // Case 4: static method with Ref arg
+  static gtsam::Pose3 Expmap(gtsam::Vector xi);
+  static gtsam::Pose3 Expmap(gtsam::Vector xi,
+                              Eigen::Ref<Eigen::MatrixXd> Hxi);
+};
+
+}

tests/test_matlab_wrapper.py

@@ -119,6 +119,85 @@ def test_matrix_view_arguments(self):
         self.assertIn('Eigen::Index m', header_content)
         self.assertIn('Stride(m, 1)', header_content)
 
+    def test_eigen_ref_jacobians(self):
+        """Test that Eigen::Ref<MatrixXd> args are treated as Jacobian outputs.
+
+        Ref<MatrixXd> arguments should not appear as inputs in the MATLAB
+        dispatch check, and should be returned as extra output arguments
+        alongside the primary return value in the generated C++ MEX code.
+        Covers: primitive inputs, zero inputs, class-type inputs, static methods.
+        See https://github.com/borglab/gtsam/issues/2492
+        """
+        file = osp.join(self.INTERFACE_DIR, 'eigen_ref.i')
+
+        wrapper = MatlabWrapper(module_name='eigen_ref',
+                                top_module_namespace=['gtsam'],
+                                ignore_classes=[''])
+
+        wrapper.wrap([file], path=self.MATLAB_ACTUAL_DIR)
+
+        cpp_file = osp.join(self.MATLAB_ACTUAL_DIR, 'eigen_ref_wrapper.cpp')
+        with open(cpp_file, 'r', encoding='UTF-8') as f:
+            cpp_content = f.read()
+
+        m_file = osp.join(self.MATLAB_ACTUAL_DIR, '+gtsam', 'Pose3.m')
+        with open(m_file, 'r', encoding='UTF-8') as f:
+            matlab_content = f.read()
+
+        # Must never emit the bogus Eigen.RefMatrixXd MATLAB class check.
+        self.assertNotIn('Eigen.RefMatrixXd', matlab_content)
+        # Must never try to unwrap Ref args from in[].
+        self.assertNotIn('unwrap_shared_ptr< Eigen::MatrixXd >', cpp_content)
+        self.assertNotIn('unwrap< Eigen::MatrixXd >', cpp_content)
+
+        # Case 1: transformFrom — primitive input (Point3) + 2 Ref args.
+        # MATLAB: only 1 varargin (point), nargout == 3.
+        self.assertIn(
+            "length(varargin) == 1 && isa(varargin{1},'double')"
+            " && size(varargin{1},1)==3 && size(varargin{1},2)==1"
+            " && nargout == 3",
+            matlab_content)
+        # C++: allocates both Ref args, passes without dereference, returns out[1]/out[2].
+        self.assertIn('Eigen::MatrixXd Hself = Eigen::MatrixXd();', cpp_content)
+        self.assertIn('Eigen::MatrixXd Hpoint = Eigen::MatrixXd();', cpp_content)
+        self.assertIn('obj->transformFrom(point,Hself,Hpoint)', cpp_content)
+        self.assertIn('out[1] = wrap< Eigen::MatrixXd >(Hself);', cpp_content)
+        self.assertIn('out[2] = wrap< Eigen::MatrixXd >(Hpoint);', cpp_content)
+        self.assertIn('checkArguments("transformFrom",nargout,nargin-1,1);', cpp_content)
+
+        # Case 2: inverse — zero real inputs + 1 Ref arg.
+        # MATLAB: length(varargin) == 0, nargout == 2.
+        self.assertIn('length(varargin) == 0 && nargout == 2', matlab_content)
+        # C++: single Ref arg allocated and returned via out[1].
+        self.assertIn('Eigen::MatrixXd H = Eigen::MatrixXd();', cpp_content)
+        self.assertIn('obj->inverse(H)', cpp_content)
+        self.assertIn('out[1] = wrap< Eigen::MatrixXd >(H);', cpp_content)
+        self.assertIn('checkArguments("inverse",nargout,nargin-1,0);', cpp_content)
+
+        # Case 3: between — class-type input (Pose3) + 2 Ref args.
+        # MATLAB: 1 varargin (pose object), nargout == 3.
+        self.assertIn(
+            "length(varargin) == 1 && isa(varargin{1},'gtsam.Pose3') && nargout == 3",
+            matlab_content)
+        # C++: pose unwrapped correctly, H1/H2 allocated and returned.
+        self.assertIn('Eigen::MatrixXd H1 = Eigen::MatrixXd();', cpp_content)
+        self.assertIn('Eigen::MatrixXd H2 = Eigen::MatrixXd();', cpp_content)
+        self.assertIn('obj->between(pose,H1,H2)', cpp_content)
+        self.assertIn('out[1] = wrap< Eigen::MatrixXd >(H1);', cpp_content)
+        self.assertIn('out[2] = wrap< Eigen::MatrixXd >(H2);', cpp_content)
+
+        # Case 4: Expmap — static method + 1 Ref arg.
+        # MATLAB: 1 varargin (xi), nargout == 2.
+        self.assertIn(
+            "length(varargin) == 1 && isa(varargin{1},'double')"
+            " && size(varargin{1},2)==1 && nargout == 2",
+            matlab_content)
+        # C++: Hxi allocated and returned via out[1], nargin not decremented (static).
+        self.assertIn('Eigen::MatrixXd Hxi = Eigen::MatrixXd();', cpp_content)
+        self.assertIn('gtsam::Pose3::Expmap(xi,Hxi)', cpp_content)
+        self.assertIn('out[1] = wrap< Eigen::MatrixXd >(Hxi);', cpp_content)
+        self.assertIn('checkArguments("gtsam::Pose3.Expmap",nargout,nargin,1);', cpp_content)
+
     def test_functions(self):
         """Test interface file with function info."""
         file = osp.join(self.INTERFACE_DIR, 'functions.i')