Merge pull request #1811 from borglab/fix/python_wrapper

dellaert · web-flow · commit 69a64ffaefdb · 2024-08-26T13:45:40.000-07:00
Fix warnings and add M1 runner
diff --git a/.github/workflows/build-linux.yml b/.github/workflows/build-linux.yml
@@ -56,7 +56,7 @@ jobs:
 
     steps:
       - name: Checkout
-        uses: actions/checkout@v3
+        uses: actions/checkout@v4
 
       - name: Install Dependencies
         run: |
diff --git a/.github/workflows/build-macos.yml b/.github/workflows/build-macos.yml
@@ -26,6 +26,7 @@ jobs:
         # See https://help.github.com/en/articles/workflow-syntax-for-github-actions.
         name: [
           macos-12-xcode-14.2,
+          macos-14-xcode-15.4,
         ]
 
         build_type: [Debug, Release]
@@ -36,9 +37,14 @@ jobs:
             compiler: xcode
             version: "14.2"
 
+          - name: macos-14-xcode-15.4
+            os: macos-14
+            compiler: xcode
+            version: "15.4"
+
     steps:
       - name: Checkout
-        uses: actions/checkout@v3
+        uses: actions/checkout@v4
 
       - name: Install Dependencies
         run: |
diff --git a/.github/workflows/build-python.yml b/.github/workflows/build-python.yml
@@ -32,6 +32,7 @@ jobs:
             ubuntu-20.04-gcc-9-tbb,
             ubuntu-20.04-clang-9,
             macos-12-xcode-14.2,
+            macos-14-xcode-15.4,
             windows-2022-msbuild,
           ]
 
@@ -59,13 +60,18 @@ jobs:
             compiler: xcode
             version: "14.2"
 
+          - name: macos-14-xcode-15.4
+            os: macos-14
+            compiler: xcode
+            version: "15.4"
+
           - name: windows-2022-msbuild
             os: windows-2022
             platform: 64
 
     steps:
       - name: Checkout
-        uses: actions/checkout@v3
+        uses: actions/checkout@v4
 
       - name: Install (Linux)
         if: runner.os == 'Linux'
@@ -162,6 +168,14 @@ jobs:
         run: |
           bash .github/scripts/python.sh -d
 
+      - name: Create virtual on MacOS
+        if: runner.os == 'macOS'
+        run: |
+          python$PYTHON_VERSION -m venv venv
+          source venv/bin/activate
+          echo "PATH=$(pwd)/venv/bin:$PATH" >> $GITHUB_ENV
+          python -m pip install --upgrade pip
+
       - name: Install Python Dependencies
         shell: bash
         run: python$PYTHON_VERSION -m pip install -r python/dev_requirements.txt
diff --git a/.github/workflows/build-special.yml b/.github/workflows/build-special.yml
@@ -83,7 +83,7 @@ jobs:
 
     steps:
       - name: Checkout
-        uses: actions/checkout@v3
+        uses: actions/checkout@v4
 
       - name: Install (Linux)
         if: runner.os == 'Linux'
diff --git a/.github/workflows/build-windows.yml b/.github/workflows/build-windows.yml
@@ -44,7 +44,7 @@ jobs:
 
     steps:
       - name: Checkout
-        uses: actions/checkout@v3
+        uses: actions/checkout@v4
 
       - name: Setup msbuild
         uses: ilammy/msvc-dev-cmd@v1
diff --git a/gtsam/3rdparty/Eigen/Eigen/src/SparseCore/TriangularSolver.h b/gtsam/3rdparty/Eigen/Eigen/src/SparseCore/TriangularSolver.h
@@ -270,11 +270,9 @@ struct sparse_solve_triangular_sparse_selector<Lhs,Rhs,Mode,UpLo,ColMajor>
       }
 
 
-      Index count = 0;
       // FIXME compute a reference value to filter zeros
       for (typename AmbiVector<Scalar,StorageIndex>::Iterator it(tempVector/*,1e-12*/); it; ++it)
       {
-        ++ count;
 //         std::cerr << "fill " << it.index() << ", " << col << "\n";
 //         std::cout << it.value() << "  ";
         // FIXME use insertBack
diff --git a/gtsam/3rdparty/Eigen/Eigen/src/SparseLU/SparseLU_heap_relax_snode.h b/gtsam/3rdparty/Eigen/Eigen/src/SparseLU/SparseLU_heap_relax_snode.h
@@ -75,8 +75,6 @@ void SparseLUImpl<Scalar,StorageIndex>::heap_relax_snode (const Index n, IndexVe
   // Identify the relaxed supernodes by postorder traversal of the etree
   Index snode_start; // beginning of a snode 
   StorageIndex k;
-  Index nsuper_et_post = 0; // Number of relaxed snodes in postordered etree 
-  Index nsuper_et = 0; // Number of relaxed snodes in the original etree 
   StorageIndex l; 
   for (j = 0; j < n; )
   {
@@ -88,7 +86,6 @@ void SparseLUImpl<Scalar,StorageIndex>::heap_relax_snode (const Index n, IndexVe
       parent = et(j);
     }
     // Found a supernode in postordered etree, j is the last column 
-    ++nsuper_et_post;
     k = StorageIndex(n);
     for (Index i = snode_start; i <= j; ++i)
       k = (std::min)(k, inv_post(i));
@@ -97,7 +94,6 @@ void SparseLUImpl<Scalar,StorageIndex>::heap_relax_snode (const Index n, IndexVe
     {
       // This is also a supernode in the original etree
       relax_end(k) = l; // Record last column 
-      ++nsuper_et; 
     }
     else 
     {
@@ -107,7 +103,6 @@ void SparseLUImpl<Scalar,StorageIndex>::heap_relax_snode (const Index n, IndexVe
         if (descendants(i) == 0) 
         {
           relax_end(l) = l;
-          ++nsuper_et;
         }
       }
     }
diff --git a/gtsam/3rdparty/metis/GKlib/pdb.c b/gtsam/3rdparty/metis/GKlib/pdb.c
@@ -131,7 +131,7 @@ that structure.
 /************************************************************************/
 pdbf *gk_readpdbfile(char *fname) { /* {{{ */
 	int i=0, res=0; 
-	char linetype[6];
+	char linetype[7];
 	int  aserial;
 	char aname[5] = "    \0";
 	char altLoc   = ' ';
diff --git a/gtsam/discrete/tests/testDiscreteConditional.cpp b/gtsam/discrete/tests/testDiscreteConditional.cpp
@@ -292,7 +292,7 @@ TEST(DiscreteConditional, choose) {
 /* ************************************************************************* */
 // Check argmax on P(C|D) and P(D), plus tie-breaking for P(B)
 TEST(DiscreteConditional, Argmax) {
-  DiscreteKey B(2, 2), C(2, 2), D(4, 2);
+  DiscreteKey C(2, 2), D(4, 2);
   DiscreteConditional B_prior(D, "1/1");
   DiscreteConditional D_prior(D, "1/3");
   DiscreteConditional C_given_D((C | D) = "1/4 1/1");
diff --git a/python/CMakeLists.txt b/python/CMakeLists.txt
@@ -263,18 +263,27 @@ if(GTSAM_UNSTABLE_BUILD_PYTHON)
 endif()
 
 # Add custom target so we can install with `make python-install`
-if (NOT WIN32) # WIN32=1 is target platform is Windows
-    add_custom_target(python-install
-        COMMAND stubgen -q -p gtsam -o ./stubs && cp -a stubs/gtsam/ gtsam && ${PYTHON_EXECUTABLE} -m pip install --user .
+# Note below we make sure to install with --user iff not in a virtualenv
+set(GTSAM_PYTHON_INSTALL_TARGET python-install)
+add_custom_target(${GTSAM_PYTHON_INSTALL_TARGET}
+        COMMAND ${PYTHON_EXECUTABLE} -c "import sys, subprocess; cmd = [sys.executable, '-m', 'pip', 'install']; has_venv = hasattr(sys, 'real_prefix') or (hasattr(sys, 'base_prefix') and sys.base_prefix != sys.prefix); cmd.append('--user' if not has_venv else ''); cmd.append('.'); subprocess.check_call([c for c in cmd if c])"
         DEPENDS ${GTSAM_PYTHON_DEPENDENCIES}
-        WORKING_DIRECTORY ${GTSAM_PYTHON_BUILD_DIRECTORY})
-else()
-    #TODO(Varun) Find equivalent cp on Windows
-    add_custom_target(python-install
-        COMMAND ${PYTHON_EXECUTABLE} -m pip install --user .
-        DEPENDS ${GTSAM_PYTHON_DEPENDENCIES}
-        WORKING_DIRECTORY ${GTSAM_PYTHON_BUILD_DIRECTORY})
-endif()
+        WORKING_DIRECTORY ${GTSAM_PYTHON_BUILD_DIRECTORY}
+        VERBATIM)
+
+# if (NOT WIN32) # WIN32=1 is target platform is Windows
+#     add_custom_target(${GTSAM_PYTHON_INSTALL_TARGET}
+#         COMMAND stubgen -q -p gtsam -o ./stubs && cp -a stubs/gtsam/ gtsam && ${PYTHON_EXECUTABLE} -m pip install --user .
+#         DEPENDS ${GTSAM_PYTHON_DEPENDENCIES}
+#         WORKING_DIRECTORY ${GTSAM_PYTHON_BUILD_DIRECTORY})
+# else()
+#     #TODO(Varun) Find equivalent cp on Windows
+#     add_custom_target(${GTSAM_PYTHON_INSTALL_TARGET}
+#         COMMAND ${PYTHON_EXECUTABLE} -m pip install --user .
+#         DEPENDS ${GTSAM_PYTHON_DEPENDENCIES}
+#         WORKING_DIRECTORY ${GTSAM_PYTHON_BUILD_DIRECTORY})
+# endif()
+
 
 # Custom make command to run all GTSAM Python tests
 add_custom_target(

Original file line number	Diff line number	Diff line change
`@@ -270,11 +270,9 @@ struct sparse_solve_triangular_sparse_selector<Lhs,Rhs,Mode,UpLo,ColMajor>`
`270`	`270`	`}`
`271`	`271`
`272`	`272`
`273`		`- Index count = 0;`
`274`	`273`	`// FIXME compute a reference value to filter zeros`
`275`	`274`	`for (typename AmbiVector<Scalar,StorageIndex>::Iterator it(tempVector/,1e-12/); it; ++it)`
`276`	`275`	`{`
`277`		`- ++ count;`
`278`	`276`	`// std::cerr << "fill " << it.index() << ", " << col << "\n";`
`279`	`277`	`// std::cout << it.value() << " ";`
`280`	`278`	`// FIXME use insertBack`
Original file line number	Diff line number	Diff line change
`@@ -75,8 +75,6 @@ void SparseLUImpl<Scalar,StorageIndex>::heap_relax_snode (const Index n, IndexVe`
`75`	`75`	`// Identify the relaxed supernodes by postorder traversal of the etree`
`76`	`76`	`Index snode_start; // beginning of a snode`
`77`	`77`	`StorageIndex k;`
`78`		`- Index nsuper_et_post = 0; // Number of relaxed snodes in postordered etree`
`79`		`- Index nsuper_et = 0; // Number of relaxed snodes in the original etree`
`80`	`78`	`StorageIndex l;`
`81`	`79`	`for (j = 0; j < n; )`
`82`	`80`	`{`
`@@ -88,7 +86,6 @@ void SparseLUImpl<Scalar,StorageIndex>::heap_relax_snode (const Index n, IndexVe`
`88`	`86`	`parent = et(j);`
`89`	`87`	`}`
`90`	`88`	`// Found a supernode in postordered etree, j is the last column`
`91`		`- ++nsuper_et_post;`
`92`	`89`	`k = StorageIndex(n);`
`93`	`90`	`for (Index i = snode_start; i <= j; ++i)`
`94`	`91`	`k = (std::min)(k, inv_post(i));`
`@@ -97,7 +94,6 @@ void SparseLUImpl<Scalar,StorageIndex>::heap_relax_snode (const Index n, IndexVe`
`97`	`94`	`{`
`98`	`95`	`// This is also a supernode in the original etree`
`99`	`96`	`relax_end(k) = l; // Record last column`
`100`		`- ++nsuper_et;`
`101`	`97`	`}`
`102`	`98`	`else`
`103`	`99`	`{`
`@@ -107,7 +103,6 @@ void SparseLUImpl<Scalar,StorageIndex>::heap_relax_snode (const Index n, IndexVe`
`107`	`103`	`if (descendants(i) == 0)`
`108`	`104`	`{`
`109`	`105`	`relax_end(l) = l;`
`110`		`- ++nsuper_et;`
`111`	`106`	`}`
`112`	`107`	`}`
`113`	`108`	`}`