From 5a7dec64d0c6ed7c42210466ff613fb5205ee2e8 Mon Sep 17 00:00:00 2001 From: Alan Morris Date: Sun, 12 Jul 2026 10:54:03 -0600 Subject: [PATCH 1/2] Ignore all-blank columns when loading projects (#1985) Copy/pasted spreadsheets often leave an orphaned header (e.g. "world_particles_femur") whose cells are all blank. Previously this created a phantom domain and blank particle-file reads, making the project fail to load. ProjectReader::load_subjects now drops any column that is blank for every subject before parsing, so orphaned all-blank headers are ignored. Applies to both the Excel and JSON readers via the shared base class. Adds ProjectTests.ignore_blank_columns_test with a fixture exercising a phantom all-blank domain and an all-blank extra column. --- Libs/Project/ProjectReader.cpp | 29 ++++++++++++++ Libs/Project/ProjectReader.h | 4 ++ Testing/ProjectTests/ProjectTests.cpp | 37 ++++++++++++++++++ .../data/project/ignore_blank_columns.swproj | 38 +++++++++++++++++++ 4 files changed, 108 insertions(+) create mode 100644 Testing/data/project/ignore_blank_columns.swproj diff --git a/Libs/Project/ProjectReader.cpp b/Libs/Project/ProjectReader.cpp index d7b4142cd84..6681b7ff260 100644 --- a/Libs/Project/ProjectReader.cpp +++ b/Libs/Project/ProjectReader.cpp @@ -3,6 +3,7 @@ #include #include +#include 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 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> subjects; bool first = true; for (auto& item : list) { diff --git a/Libs/Project/ProjectReader.h b/Libs/Project/ProjectReader.h index 8b24f576c51..8c085d91952 100644 --- a/Libs/Project/ProjectReader.h +++ b/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_; diff --git a/Testing/ProjectTests/ProjectTests.cpp b/Testing/ProjectTests/ProjectTests.cpp index c3ad705ba96..0646f1a863b 100644 --- a/Testing/ProjectTests/ProjectTests.cpp +++ b/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->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); + } +} + //--------------------------------------------------------------------------- TEST(ProjectTests, conversion_to_excel_test) { TestUtils::Instance().prep_temp(std::string(TEST_DATA_DIR) + "/project", "project_convert_to_excel_test"); diff --git a/Testing/data/project/ignore_blank_columns.swproj b/Testing/data/project/ignore_blank_columns.swproj new file mode 100644 index 00000000000..e34707b3b2c --- /dev/null +++ b/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": "" + } + ] +} From c7d382485c229b583a97549685d7e76643753d3d Mon Sep 17 00:00:00 2001 From: Alan Morris Date: Mon, 13 Jul 2026 00:23:14 -0600 Subject: [PATCH 2/2] Fix formatting --- Libs/Optimize/OptimizeParameters.cpp | 94 +++++++++++++--------------- Studio/Optimize/OptimizeTool.cpp | 3 +- 2 files changed, 47 insertions(+), 50 deletions(-) diff --git a/Libs/Optimize/OptimizeParameters.cpp b/Libs/Optimize/OptimizeParameters.cpp index 4a3015736a9..5a55c138fa1 100644 --- a/Libs/Optimize/OptimizeParameters.cpp +++ b/Libs/Optimize/OptimizeParameters.cpp @@ -5,14 +5,14 @@ #include #include #include +#include +#include #include #include #include #include #include -#include -#include #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 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(0, mesh_work_indices.size()), - [&](const tbb::blocked_range& 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( - item.poly_data, geodesics_on_main, - static_cast(geodesic_cache_multiplier)); - - // Create the Mesh wrapper and compute surface area - auto sw_mesh = std::make_shared(surface->get_polydata()); - double surface_area = sw_mesh->getSurfaceArea(); - - // Create remeshed Surface for geodesics if needed - std::shared_ptr 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( - mesh_copy.getVTKMesh(), geodesics_enabled, - static_cast(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(done) / static_cast(total) * 100.0; - SW_PROGRESS(progress, "{}: {} / {}", loading_message, done, total); - } - }); + tbb::parallel_for( + tbb::blocked_range(0, mesh_work_indices.size()), [&](const tbb::blocked_range& 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(item.poly_data, geodesics_on_main, + static_cast(geodesic_cache_multiplier)); + + // Create the Mesh wrapper and compute surface area + auto sw_mesh = std::make_shared(surface->get_polydata()); + double surface_area = sw_mesh->getSurfaceArea(); + + // Create remeshed Surface for geodesics if needed + std::shared_ptr 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(mesh_copy.getVTKMesh(), geodesics_enabled, + static_cast(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(done) / static_cast(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); diff --git a/Studio/Optimize/OptimizeTool.cpp b/Studio/Optimize/OptimizeTool.cpp index a79a39b009d..7e327cf7a88 100644 --- a/Studio/Optimize/OptimizeTool.cpp +++ b/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