Volume Enhanced DeepSDF by ErendiroPedro · Pull Request #2 · paulkry/topology-control

ErendiroPedro · 2025-07-21T17:10:19Z

Introduces a enhanced dataset with sdf and volumes
and volume deep sdf to predict both tasks
visualize sdf mesh

Copilot

Pull Request Overview

This PR introduces Volume Enhanced DeepSDF, a comprehensive enhancement to the existing DeepSDF architecture that adds dual prediction capabilities for both signed distance functions (SDF) and shape volumes. The implementation introduces several new architectural components, enhanced datasets, and improved memory management throughout the training pipeline.

Key Changes:

Implementation of VolumeDeepSDF architecture with dual SDF/volume prediction heads
Enhanced dataset classes (VolumeSDFDataset) that incorporate shape volume information
Memory-optimized training pipeline with device fallback capabilities
Improved evaluation system supporting both SDF and volume metrics

Reviewed Changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
src/CPipelineOrchestrator.py	Major refactoring with enhanced training pipeline, memory management, and comprehensive artifact tracking
src/CModelTrainer.py	Complete rewrite with dual-loss training, memory optimization, and enhanced dataset support
src/CGeometryUtils.py	Added volume computation utilities for mesh volume calculation
src/CEvaluator.py	Enhanced evaluation supporting both SDF and volume model types
src/CDataProcessor.py	Extended data processing with volume-aware dataset generation
src/CArchitectureManager.py	Added VolumeDeepSDF model with sophisticated dual-head architecture
config/config.yaml	Updated configuration for volume-enhanced training parameters
main.py	Updated method call to new pipeline interface

Comments suppressed due to low confidence (1)

src/CPipelineOrchestrator.py:152

[nitpick] The comment mentions 'SDF training with volumes' but the method name 'generate_sdf_dataset' doesn't clearly indicate it handles volumes. Consider updating the comment to be more specific about volume-enhanced SDF datasets.

        # Generate dataset for SDF training with volumes

Copilot · 2025-07-21T17:12:26Z

        experiment_summary = self.artifact_manager.get_experiment_summary()
        experiment_id = experiment_summary['experiment_id']
-        artifacts_base = self.config["artifacts_config"]["save_artifacts_to"]
+        artifacts_base = self.artifact_manager.artifacts_path  # FIXED: Use artifacts_path instead of artifacts_base


[nitpick] The comment 'FIXED:' suggests this was a bug fix, but the variable name 'artifacts_base' is inconsistent with the actual property being accessed. Consider renaming the variable to match the property name for clarity.

Suggested change

artifacts_base = self.artifact_manager.artifacts_path # FIXED: Use artifacts_path instead of artifacts_base

artifacts_path = self.artifact_manager.artifacts_path # FIXED: Use artifacts_path instead of artifacts_base

Copilot · 2025-07-21T17:12:27Z

+        # Get file list for this split
+        self.files = dataset_info[f'{split}_files']
+
+        # FIXED: Initialize latent vectors properly as leaf tensors


[nitpick] Multiple 'FIXED:' comments throughout the code indicate recent bug fixes. Consider removing these comments once the fixes are verified and tested, as they add noise to the codebase.

Suggested change

# FIXED: Initialize latent vectors properly as leaf tensors

Copilot · 2025-07-21T17:12:27Z

+        # Volume Loss (MAE) - CHANGED from MSE to MAE
+        volume_pred = predictions['volume']
+        volume_target = targets['volume']
+        volume_loss = torch.nn.functional.l1_loss(volume_pred, volume_target)  # MAE instead of MSE


The choice between MAE and MSE for volume loss is important and should be configurable. Consider adding this as a parameter in the loss configuration rather than hardcoding it.

Copilot · 2025-07-21T17:12:27Z

+                    {"params": [latent_vectors], "lr": lr_latent}  # Wrap in list
                ])
            elif self.optimizer_name.lower() == 'adamw':
                optimizer = optim.AdamW([
                    {"params": model.parameters(), "lr": lr_net},
-                    {"params": latent_vectors, "lr": lr_latent}
+                    {"params": [latent_vectors], "lr": lr_latent}  # Wrap in list
                ])
            else:
                # Default to Adam for SDF training
                optimizer = optim.Adam([
                    {"params": model.parameters(), "lr": lr_net},
-                    {"params": latent_vectors, "lr": lr_latent}
+                    {"params": [latent_vectors], "lr": lr_latent}  # Wrap in list


The repeated pattern of wrapping latent_vectors in a list occurs multiple times. Consider extracting this into a helper method to reduce code duplication and improve maintainability.

Copilot · 2025-07-21T17:12:28Z

+        raise ValueError("Mesh is not watertight, cannot compute volume")
+
+    mesh = trimesh.Trimesh(vertices=vertices, faces=faces)
+    volume = abs(mesh.volume)


Using abs() to ensure positive volume handles negative volumes from incorrect mesh orientation, but this might mask actual mesh issues. Consider logging a warning when the original volume is negative to help with debugging mesh problems.

Suggested change

volume = abs(mesh.volume)

volume = mesh.volume

if volume < 0:

print("Warning: Mesh volume is negative. This may indicate an issue with mesh orientation or integrity.")

volume = abs(volume)

Copilot · 2025-07-21T17:12:28Z

+        # Skip connection adds input_dim to layer_size: 256 + 19 = 275
+        skip_concat_dim = self.layer_size + self.input_dim  # 256 + 19 = 275


Hardcoded dimension calculations in comments (e.g., '256 + 19 = 275') can become outdated when configuration changes. Consider removing these specific numbers from comments to avoid maintenance issues.

Suggested change

# Skip connection adds input_dim to layer_size: 256 + 19 = 275

skip_concat_dim = self.layer_size + self.input_dim # 256 + 19 = 275

# Skip connection adds input_dim to layer_size

skip_concat_dim = self.layer_size + self.input_dim

Copilot · 2025-07-21T17:12:28Z

+        volume_features = volume_features.view(batch_size, num_coords, -1)
+
+        # Deep Sets aggregation (permutation invariant)
+        aggregated_features, _ = torch.max(volume_features, dim=1)  # [batch_size, volume_hidden_dim]


Using max pooling for Deep Sets aggregation is one approach, but the choice of aggregation function (max, mean, sum) significantly affects model performance. Consider making this configurable or documenting why max pooling was chosen over other options.

Suggested change

aggregated_features, _ = torch.max(volume_features, dim=1) # [batch_size, volume_hidden_dim]

if self.aggregation_method == 'max':

aggregated_features, _ = torch.max(volume_features, dim=1) # [batch_size, volume_hidden_dim]

elif self.aggregation_method == 'mean':

aggregated_features = torch.mean(volume_features, dim=1) # [batch_size, volume_hidden_dim]

elif self.aggregation_method == 'sum':

aggregated_features = torch.sum(volume_features, dim=1) # [batch_size, volume_hidden_dim]

Copilot · 2025-07-21T17:12:28Z

+            # NEW: If using same shapes and mesh already processed for training, 
+            # create symlinks or reference same files to save processing time
+            if self.use_same_shapes_for_val and mesh_name in processed_train_meshes:
+                print(f"  [VAL] [Same Shape] Reusing training data for {mesh_name}")
+
+                # Find corresponding train files
+                train_idx = processing_results['train_files'].index(mesh_name)
+                train_points_file = processing_results['point_cloud_files']['train'][train_idx]
+                train_distances_file = processing_results['signed_distance_files']['train'][train_idx]
+
+                # For validation, we can either:
+                # 1. Use same files (efficient but same exact samples)
+                # 2. Process again with different random seed (different samples, more realistic validation)
+
+                # Option 1: Reuse same files (faster)
+                val_points_file = train_points_file
+                val_distances_file = train_distances_file
+
+                # Option 2: Process with different sampling (uncomment if preferred)
+                # result = self._process_single_mesh(mesh_path, 'val')
+                # if result is not None:
+                #     val_points_file = result['points_file']
+                #     val_distances_file = result['distances_file']
+                # else:
+                #     continue
+
+                processing_results['val_files'].append(mesh_name)
+                processing_results['point_cloud_files']['val'].append(val_points_file)
+                processing_results['signed_distance_files']['val'].append(val_distances_file)
+
+                # Don't double-count points if reusing same files
+                if val_points_file != train_points_file:
+                    points_data = np.load(val_points_file)
+                    processing_results['total_points_generated'] += len(points_data)
+
            else:
-                # Track failed files
-                mesh_name = os.path.splitext(os.path.basename(mesh_path))[0]
-                processing_results['corrupted_files'].append(mesh_name)
-                processing_results['skipped_files'].append(f"{mesh_name} (val)")
+                # Process normally for different shapes or if not reusing
+                result = self._process_single_mesh(mesh_path, 'val')
+                if result is not None:
+                    if mesh_name not in processing_results['processed_files']:
+                        processing_results['processed_files'].append(result['mesh_name'])
+                    processing_results['val_files'].append(result['mesh_name'])
+                    processing_results['point_cloud_files']['val'].append(result['points_file'])
+                    processing_results['signed_distance_files']['val'].append(result['distances_file'])
+                    processing_results['total_points_generated'] += result['num_points']
+                else:
+                    # Track failed files
+                    processing_results['corrupted_files'].append(mesh_name)
+                    processing_results['skipped_files'].append(f"{mesh_name} (val)")


The logic for handling same shapes for validation is complex and contains commented-out alternative approaches. Consider extracting this into a separate method to improve readability and make the different strategies more explicit.

Copilot · 2025-07-21T17:12:29Z

+    dropout_p: 0.2
+
+  volumedeepsdf:
+    z_dim: 2


The z_dim for volumedeepsdf is set to 2, which is unusually small for latent space dimensionality and inconsistent with the deepsdf configuration (z_dim: 16). This likely should be 16 to match the dataset configuration and provide sufficient latent capacity.

Suggested change

z_dim: 2

z_dim: 16

ErendiroPedro added 4 commits July 21, 2025 17:48

add feature for volumedeepsdf

1cec699

update main.py

c3b0689

update requirements.txt

70a4b78

update config

a99aaa4

ErendiroPedro requested review from NoginaDaria, Copilot and marilevay July 21, 2025 17:10

Copilot AI reviewed Jul 21, 2025

View reviewed changes

ErendiroPedro requested review from YuanyuanTao and paulkry July 22, 2025 09:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Volume Enhanced DeepSDF#2

Volume Enhanced DeepSDF#2
ErendiroPedro wants to merge 4 commits intomainfrom
volumedeepsdf

ErendiroPedro commented Jul 21, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jul 21, 2025

Uh oh!

Copilot AI Jul 21, 2025

Uh oh!

Copilot AI Jul 21, 2025

Uh oh!

Copilot AI Jul 21, 2025

Uh oh!

Copilot AI Jul 21, 2025

Uh oh!

Copilot AI Jul 21, 2025

Uh oh!

Copilot AI Jul 21, 2025

Uh oh!

Copilot AI Jul 21, 2025

Uh oh!

Copilot AI Jul 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	artifacts_base = self.artifact_manager.artifacts_path # FIXED: Use artifacts_path instead of artifacts_base
	artifacts_path = self.artifact_manager.artifacts_path # FIXED: Use artifacts_path instead of artifacts_base

-    volume = abs(mesh.volume)
+    volume = mesh.volume
+    if volume < 0:
+        print("Warning: Mesh volume is negative. This may indicate an issue with mesh orientation or integrity.")
+    volume = abs(volume)

		# Skip connection adds input_dim to layer_size: 256 + 19 = 275
		skip_concat_dim = self.layer_size + self.input_dim # 256 + 19 = 275

-        aggregated_features, _ = torch.max(volume_features, dim=1)  # [batch_size, volume_hidden_dim]
+        if self.aggregation_method == 'max':
+            aggregated_features, _ = torch.max(volume_features, dim=1)  # [batch_size, volume_hidden_dim]
+        elif self.aggregation_method == 'mean':
+            aggregated_features = torch.mean(volume_features, dim=1)  # [batch_size, volume_hidden_dim]
+        elif self.aggregation_method == 'sum':
+            aggregated_features = torch.sum(volume_features, dim=1)  # [batch_size, volume_hidden_dim]

Conversation

ErendiroPedro commented Jul 21, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Key Changes:

Reviewed Changes

Uh oh!

Copilot AI Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants