Ignore all-blank columns when loading projects (#1985)

Libs/Optimize/OptimizeParameters.cpp

@@ -5,14 +5,14 @@
 #include <Mesh/MeshUtils.h>
 #include <Particles/ParticleFile.h>
 #include <Utils/StringUtils.h>
+#include <tbb/blocked_range.h>
+#include <tbb/parallel_for.h>
 
 #include <atomic>
 #include <boost/algorithm/string.hpp>
 #include <boost/filesystem.hpp>
 #include <functional>
 #include <optional>
-#include <tbb/parallel_for.h>
-#include <tbb/blocked_range.h>
 
 #include "Libs/Optimize/Domain/Surface.h"
 #include "Optimize.h"
@@ -123,7 +123,7 @@ OptimizeParameters::OptimizeParameters(ProjectHandle project) {
                                          Keys::early_stopping_ema_alpha,
                                          Keys::early_stopping_enabled,
                                          Keys::early_stopping_warmup_iters,
-                                         Keys::early_stopping_enable_logging };
+                                         Keys::early_stopping_enable_logging};
 
   std::vector<std::string> to_remove;
 
@@ -886,47 +886,45 @@ bool OptimizeParameters::set_up_optimize(Optimize* optimize) {
     std::string loading_message = remeshing ? "Loading and remeshing meshes" : "Loading meshes";
     SW_PROGRESS(0, "{}: 0 / {}", loading_message, total);
 
-    tbb::parallel_for(tbb::blocked_range<size_t>(0, mesh_work_indices.size()),
-                      [&](const tbb::blocked_range<size_t>& r) {
-                        for (size_t i = r.begin(); i < r.end(); ++i) {
-                          auto& item = work_items[mesh_work_indices[i]];
-
-                          // When we'll precompute geodesics on a remeshed copy, skip the (expensive)
-                          // geodesic precompute on the full-resolution surface — it's only used for
-                          // cell-locator / snap / normals / geodesic_walk, none of which need it.
-                          bool geodesics_on_main = geodesics_enabled && !remeshing;
-                          auto surface = std::make_shared<shapeworks::Surface>(
-                              item.poly_data, geodesics_on_main,
-                              static_cast<size_t>(geodesic_cache_multiplier));
-
-                          // Create the Mesh wrapper and compute surface area
-                          auto sw_mesh = std::make_shared<Mesh>(surface->get_polydata());
-                          double surface_area = sw_mesh->getSurfaceArea();
-
-                          // Create remeshed Surface for geodesics if needed
-                          std::shared_ptr<shapeworks::Surface> geodesics_surface;
-                          if (!remeshing) {
-                            geodesics_surface = surface;
-                          } else {
-                            auto poly_copy = surface->get_polydata();
-                            Mesh mesh_copy(poly_copy);
-                            // remeshPercent takes a 0–1 fraction; project value is 0–100
-                            mesh_copy.remeshPercent(effective_remesh_percent / 100.0, 1.0);
-                            geodesics_surface = std::make_shared<shapeworks::Surface>(
-                                mesh_copy.getVTKMesh(), geodesics_enabled,
-                                static_cast<size_t>(geodesic_cache_multiplier));
-                          }
-
-                          item.surface = surface;
-                          item.geodesics_surface = geodesics_surface;
-                          item.sw_mesh = sw_mesh;
-                          item.surface_area = surface_area;
-
-                          int done = ++completed;
-                          double progress = static_cast<double>(done) / static_cast<double>(total) * 100.0;
-                          SW_PROGRESS(progress, "{}: {} / {}", loading_message, done, total);
-                        }
-                      });
+    tbb::parallel_for(
+        tbb::blocked_range<size_t>(0, mesh_work_indices.size()), [&](const tbb::blocked_range<size_t>& r) {
+          for (size_t i = r.begin(); i < r.end(); ++i) {
+            auto& item = work_items[mesh_work_indices[i]];
+
+            // When we'll precompute geodesics on a remeshed copy, skip the (expensive)
+            // geodesic precompute on the full-resolution surface — it's only used for
+            // cell-locator / snap / normals / geodesic_walk, none of which need it.
+            bool geodesics_on_main = geodesics_enabled && !remeshing;
+            auto surface = std::make_shared<shapeworks::Surface>(item.poly_data, geodesics_on_main,
+                                                                 static_cast<size_t>(geodesic_cache_multiplier));
+
+            // Create the Mesh wrapper and compute surface area
+            auto sw_mesh = std::make_shared<Mesh>(surface->get_polydata());
+            double surface_area = sw_mesh->getSurfaceArea();
+
+            // Create remeshed Surface for geodesics if needed
+            std::shared_ptr<shapeworks::Surface> geodesics_surface;
+            if (!remeshing) {
+              geodesics_surface = surface;
+            } else {
+              auto poly_copy = surface->get_polydata();
+              Mesh mesh_copy(poly_copy);
+              // remeshPercent takes a 0–1 fraction; project value is 0–100
+              mesh_copy.remeshPercent(effective_remesh_percent / 100.0, 1.0);
+              geodesics_surface = std::make_shared<shapeworks::Surface>(mesh_copy.getVTKMesh(), geodesics_enabled,
+                                                                        static_cast<size_t>(geodesic_cache_multiplier));
+            }
+
+            item.surface = surface;
+            item.geodesics_surface = geodesics_surface;
+            item.sw_mesh = sw_mesh;
+            item.surface_area = surface_area;
+
+            int done = ++completed;
+            double progress = static_cast<double>(done) / static_cast<double>(total) * 100.0;
+            SW_PROGRESS(progress, "{}: {} / {}", loading_message, done, total);
+          }
+        });
   }
 
   // === Phase C: Sequential registration (fast) ===
@@ -1134,14 +1132,12 @@ EarlyStoppingConfig OptimizeParameters::get_early_stopping_config() {
   config.frequency = params_.get(Keys::early_stopping_frequency, 1000);
   config.window_size = params_.get(Keys::early_stopping_window, 5);
   config.threshold = params_.get(Keys::early_stopping_threshold, 0.0001);
-  std::string strategy =
-      params_.get(Keys::early_stopping_strategy, "relative_difference");
+  std::string strategy = params_.get(Keys::early_stopping_strategy, "relative_difference");
   if (strategy == "exponential_moving_average") {
     config.strategy = EarlyStoppingStrategy::ExponentialMovingAverage;
-  } else if (strategy == "relative_difference"){
+  } else if (strategy == "relative_difference") {
     config.strategy = EarlyStoppingStrategy::RelativeDifference;
-  }
-  else {
+  } else {
     throw std::invalid_argument("Invalid strategy for early stopping " + strategy);
   }
   config.ema_alpha = params_.get(Keys::early_stopping_ema_alpha, 0.2);

Libs/Project/ProjectReader.cpp

@@ -3,6 +3,7 @@
 #include <StringUtils.h>
 
 #include <algorithm>
+#include <set>
 
 namespace shapeworks {
 
@@ -13,8 +14,36 @@ using namespace project::types;
 //---------------------------------------------------------------------------
 ProjectReader::ProjectReader(Project& project) : project_(project) {}
 
+//---------------------------------------------------------------------------
+void ProjectReader::remove_blank_columns(StringMapList& list) {
+  // Determine which columns have a value in at least one subject.
+  std::set<std::string> non_blank;
+  for (const auto& item : list) {
+    for (const auto& [key, value] : item) {
+      if (!value.empty()) {
+        non_blank.insert(key);
+      }
+    }
+  }
+
+  // Remove columns that are blank for every subject.  Copy/pasted spreadsheets
+  // often leave an orphan header (e.g. "world_particles_femur") with no values;
+  // keeping it would create a phantom domain and blank file reads. (#1985)
+  for (auto& item : list) {
+    for (auto it = item.begin(); it != item.end();) {
+      if (non_blank.find(it->first) == non_blank.end()) {
+        it = item.erase(it);
+      } else {
+        ++it;
+      }
+    }
+  }
+}
+
 //---------------------------------------------------------------------------
 void ProjectReader::load_subjects(StringMapList list) {
+  remove_blank_columns(list);
+
   std::vector<std::shared_ptr<Subject>> subjects;
   bool first = true;
   for (auto& item : list) {

Libs/Project/ProjectReader.h

@@ -41,6 +41,10 @@ class ProjectReader {
 
   static StringList get_keys(StringMap map);
 
+  //! Remove columns that are blank for every subject (e.g. an orphaned header
+  //! left over from a copy/pasted spreadsheet).
+  static void remove_blank_columns(StringMapList &list);
+
   bool contains(StringMap map, std::string key);
 
   Project &project_;

Studio/Optimize/OptimizeTool.cpp

@@ -110,7 +110,8 @@ OptimizeTool::OptimizeTool(Preferences& prefs, Telemetry& telemetry) : preferenc
   ui_->shared_boundary_weight->setToolTip("Weight of shared boundary optimization");
   ui_->sampling_scale->setToolTip("Enable sampling gradient scaling");
   ui_->sampling_auto_scale->setToolTip("Automatically scale sampling gradient based on surface area");
-  ui_->sampling_scale_value->setToolTip("Scale multiplier for sampling gradient (applied on top of auto-scale when enabled)");
+  ui_->sampling_scale_value->setToolTip(
+      "Scale multiplier for sampling gradient (applied on top of auto-scale when enabled)");
   ui_->use_disentangled_ssm->setToolTip("Use disentangled Optimization technique to build spatiotemporal SSM.");
 
   // hidden for 6.5 release

Testing/ProjectTests/ProjectTests.cpp

@@ -51,6 +51,43 @@ TEST(ProjectTests, conversion_to_json_test) {
   compare_projects(project_compare1, project_compare2);
 }
 
+//---------------------------------------------------------------------------
+// #1985: columns that are blank for every subject (e.g. an orphan "world_particles_ghost"
+// header left over from a copy/pasted spreadsheet) should be ignored, not turned into a
+// phantom domain or a blank file read.
+TEST(ProjectTests, ignore_blank_columns_test) {
+  TestUtils::Instance().prep_temp(std::string(TEST_DATA_DIR) + "/project", "project_ignore_blank_columns_test");
+
+  ProjectHandle project = std::make_shared<Project>();
+  project->load("ignore_blank_columns.swproj");
+
+  // the all-blank "ghost" domain should be dropped, leaving only the real domains
+  auto domain_names = project->get_domain_names();
+  ASSERT_EQ(domain_names.size(), 2);
+  EXPECT_EQ(domain_names[0], "femur");
+  EXPECT_EQ(domain_names[1], "pelvis");
+
+  auto subjects = project->get_subjects();
+  ASSERT_EQ(subjects.size(), 2);
+  for (auto& subject : subjects) {
+    EXPECT_EQ(subject->get_number_of_domains(), 2);
+
+    // no blank filenames should be produced from the ghost columns
+    EXPECT_EQ(subject->get_original_filenames().size(), 2);
+    EXPECT_EQ(subject->get_groomed_filenames().size(), 2);
+    EXPECT_EQ(subject->get_local_particle_filenames().size(), 2);
+    ASSERT_EQ(subject->get_world_particle_filenames().size(), 2);
+    for (const auto& filename : subject->get_world_particle_filenames()) {
+      EXPECT_FALSE(filename.empty());
+    }
+
+    // the all-blank extra column should be ignored
+    EXPECT_EQ(subject->get_extra_values().count("custom_blank"), 0u);
+    // the populated group column is preserved
+    EXPECT_EQ(subject->get_group_values().count("side"), 1u);
+  }
+}
+
 //---------------------------------------------------------------------------
 // #2455: after a failed/aborted optimization, the project should not be left referencing
 // particle files that were never written. clear_particle_filenames() clears them for

Testing/data/project/ignore_blank_columns.swproj

@@ -0,0 +1,38 @@
+{
+    "data": [
+        {
+            "name": "subject_01",
+            "shape_femur": "01_femur.nrrd",
+            "shape_pelvis": "01_pelvis.ply",
+            "shape_ghost": "",
+            "groomed_femur": "groomed/01_femur_DT.nrrd",
+            "groomed_pelvis": "groomed/01_pelvis_groomed.vtk",
+            "groomed_ghost": "",
+            "local_particles_femur": "particles/01_femur_local.particles",
+            "local_particles_pelvis": "particles/01_pelvis_local.particles",
+            "local_particles_ghost": "",
+            "world_particles_femur": "particles/01_femur_world.particles",
+            "world_particles_pelvis": "particles/01_pelvis_world.particles",
+            "world_particles_ghost": "",
+            "group_side": "left",
+            "custom_blank": ""
+        },
+        {
+            "name": "subject_02",
+            "shape_femur": "02_femur.nrrd",
+            "shape_pelvis": "02_pelvis.ply",
+            "shape_ghost": "",
+            "groomed_femur": "groomed/02_femur_DT.nrrd",
+            "groomed_pelvis": "groomed/02_pelvis_groomed.vtk",
+            "groomed_ghost": "",
+            "local_particles_femur": "particles/02_femur_local.particles",
+            "local_particles_pelvis": "particles/02_pelvis_local.particles",
+            "local_particles_ghost": "",
+            "world_particles_femur": "particles/02_femur_world.particles",
+            "world_particles_pelvis": "particles/02_pelvis_world.particles",
+            "world_particles_ghost": "",
+            "group_side": "right",
+            "custom_blank": ""
+        }
+    ]
+}