From c20b7b29888709cfc21a3fdc5256c0b7a8d6a846 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Nov 2025 17:01:17 +0000 Subject: [PATCH 1/3] Remove all backward compatibility requirements from refactoring plan - Remove backward compatibility mentions from migration steps - Remove backward compatibility from success criteria - Remove feature flags for backward compatibility from mitigation strategies - Remove deprecation warnings from mitigation strategies - Focus on clean, modern codebase without legacy concerns --- REFACTORING_PLAN.md | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/REFACTORING_PLAN.md b/REFACTORING_PLAN.md index c709ee8c..5ebe0788 100644 --- a/REFACTORING_PLAN.md +++ b/REFACTORING_PLAN.md @@ -263,15 +263,13 @@ connectomics/lightning/ 1. Create new module files 2. Move functionality in logical chunks 3. Update imports in `lit_model.py` -4. Maintain backward compatibility (public API unchanged) -5. Add integration tests for each module -6. Update documentation +4. Add integration tests for each module +5. Update documentation **Success Criteria:** - [ ] Each file < 500 lines - [ ] Clear separation of concerns - [ ] All existing tests pass -- [ ] Public API unchanged (backward compatible) - [ ] Documentation updated --- @@ -398,7 +396,6 @@ class DataConfig: - [x] `deep_supervision_clamp_min: float` (default: -20.0) - [x] `deep_supervision_clamp_max: float` (default: 20.0) - [x] Validation logic with warning for insufficient weights -- [x] Backward compatible (defaults match old behavior) - [ ] Other hardcoded values (target interpolation, rejection sampling) - Future work **Status:** ✅ Phase 2.3 (Deep Supervision) completed. Users can now customize deep supervision weights and clamping ranges via config. @@ -490,14 +487,12 @@ def build_test_transforms(cfg: Config, keys: list[str] = None) -> Compose: - File size reduced from 791 to 727 lines (-64 lines, ~8% reduction) - Eliminated ~80% code duplication - Single source of truth for shared transform logic -- Backward compatible (same public API) **Action Items:** - [x] Extract shared logic into `_build_eval_transforms_impl()` - [x] Identify val/test-specific differences (4 key differences) - [x] Create mode-specific branching with clear comments - [x] Keep wrapper functions for API compatibility -- [x] Backward compatible (public API unchanged) **Status:** ✅ Phase 2.5 complete. Code duplication eliminated while preserving all functionality. @@ -986,10 +981,8 @@ See Priority 1.3 above for full details. ### Mitigation Strategies 1. **Comprehensive testing** before and after each change -2. **Feature flags** for backward compatibility -3. **Deprecation warnings** before removal -4. **Rollback plan** for each phase -5. **User communication** via release notes +2. **Rollback plan** for each phase +3. **User communication** via release notes --- From 9638774b80100f10d77bdb96fe15d920330cf061 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Nov 2025 17:07:54 +0000 Subject: [PATCH 2/3] Remove all backward compatibility code from codebase Removes backward compatibility code and comments to maintain a clean, modern codebase: **Config System (callbacks.py):** - Remove old config style support for checkpoint callbacks - Remove old config style support for early_stopping callbacks - Keep only modern unified config style (cfg.monitor.*) **Lightning Model (lit_model.py):** - Remove legacy parameter conversion for decode_binary_contour_distance_watershed - Remove legacy intensity_scale/output_scale fallbacks - Keep only modern config names (intensity_scale, intensity_dtype) **Transform Builders (build.py):** - Remove backward compatibility comments - Remove legacy image_transform.resize fallback - Keep only modern data_transform.resize path **Loss Functions (regularization.py):** - Remove backward compatibility aliases (BinaryReg, FgDTConsistency, etc.) **Utilities (visualizer.py):** - Remove legacy create_visualizer() factory function **Config (hydra_config.py, hydra_utils.py):** - Remove deprecated config field comments - Remove legacy path comments **Impact:** - 7 files modified - 92 lines removed - Clean, focused codebase without legacy support --- connectomics/config/hydra_config.py | 8 ++--- connectomics/config/hydra_utils.py | 6 ++-- connectomics/data/augment/build.py | 11 +++---- connectomics/lightning/callbacks.py | 34 ++------------------- connectomics/lightning/lit_model.py | 35 ++++------------------ connectomics/models/loss/regularization.py | 8 ----- connectomics/utils/visualizer.py | 6 ---- 7 files changed, 16 insertions(+), 92 deletions(-) diff --git a/connectomics/config/hydra_config.py b/connectomics/config/hydra_config.py index d49220ad..31c4edee 100644 --- a/connectomics/config/hydra_config.py +++ b/connectomics/config/hydra_config.py @@ -392,9 +392,7 @@ class DataConfig: default_factory=list ) # Axis permutation for training data (e.g., [2,1,0] for xyz->zyx) val_transpose: List[int] = field(default_factory=list) # Axis permutation for validation data - test_transpose: List[int] = field( - default_factory=list - ) # Axis permutation for test data (deprecated, use inference.data.test_transpose) + test_transpose: List[int] = field(default_factory=list) # Axis permutation for test data # Dataset statistics (for auto-planning) target_spacing: Optional[List[float]] = None # Target voxel spacing [z, y, x] in mm @@ -868,9 +866,7 @@ class TestTimeAugmentationConfig: flip_axes: Any = ( None # TTA flip strategy: "all" (8 flips), null (no aug), or list like [[0], [1], [2]] ) - act: Optional[str] = ( - None # Single activation for all channels: 'softmax', 'sigmoid', 'tanh', None (deprecated, use channel_activations) - ) + act: Optional[str] = None # Single activation for all channels: 'softmax', 'sigmoid', 'tanh', None channel_activations: Optional[List[Any]] = ( None # Per-channel activations: [[start_ch, end_ch, 'activation'], ...] e.g., [[0, 2, 'softmax'], [2, 3, 'sigmoid'], [3, 4, 'tanh']] ) diff --git a/connectomics/config/hydra_utils.py b/connectomics/config/hydra_utils.py index c546d3ab..df9bdb82 100644 --- a/connectomics/config/hydra_utils.py +++ b/connectomics/config/hydra_utils.py @@ -244,8 +244,8 @@ def resolve_data_paths(cfg: Config) -> Config: Supported paths: - Training: cfg.data.train_path + cfg.data.train_image/train_label/train_mask - Validation: cfg.data.val_path + cfg.data.val_image/val_label/val_mask - - Testing (legacy): cfg.data.test_path + cfg.data.test_image/test_label/test_mask - - Inference (primary): cfg.inference.data.test_path + cfg.inference.data.test_image/test_label/test_mask + - Testing: cfg.data.test_path + cfg.data.test_image/test_label/test_mask + - Inference: cfg.inference.data.test_path + cfg.inference.data.test_image/test_label/test_mask Args: cfg: Config object to resolve paths for @@ -316,7 +316,7 @@ def _combine_path(base_path: str, file_path: Optional[Union[str, List[str]]]) -> cfg.data.val_mask = _combine_path(cfg.data.val_path, cfg.data.val_mask) cfg.data.val_json = _combine_path(cfg.data.val_path, cfg.data.val_json) - # Resolve test paths (legacy support for cfg.data.test_path) + # Resolve test paths if cfg.data.test_path: cfg.data.test_image = _combine_path(cfg.data.test_path, cfg.data.test_image) cfg.data.test_label = _combine_path(cfg.data.test_path, cfg.data.test_label) diff --git a/connectomics/data/augment/build.py b/connectomics/data/augment/build.py index 7404e52f..15aca44b 100644 --- a/connectomics/data/augment/build.py +++ b/connectomics/data/augment/build.py @@ -73,7 +73,7 @@ def build_train_transforms( # Load images first (unless using pre-cached dataset) if not skip_loading: # Use appropriate loader based on dataset type - dataset_type = getattr(cfg.data, "dataset_type", "volume") # Default to volume for backward compatibility + dataset_type = getattr(cfg.data, "dataset_type", "volume") if dataset_type == "filename": # For filename-based datasets (PNG, JPG, etc.), use MONAI's LoadImaged @@ -94,12 +94,9 @@ def build_train_transforms( transforms.append(ApplyVolumetricSplitd(keys=keys)) # Apply resize if configured (before cropping) - # Check data_transform first (new), then fall back to image_transform.resize (legacy) resize_size = None if hasattr(cfg.data, "data_transform") and hasattr(cfg.data.data_transform, "resize") and cfg.data.data_transform.resize is not None: resize_size = cfg.data.data_transform.resize - elif hasattr(cfg.data.image_transform, "resize") and cfg.data.image_transform.resize is not None: - resize_size = cfg.data.image_transform.resize if resize_size: # Use bilinear for images, nearest for labels/masks @@ -247,7 +244,7 @@ def _build_eval_transforms_impl( transforms = [] # Load images first - use appropriate loader based on dataset type - dataset_type = getattr(cfg.data, "dataset_type", "volume") # Default to volume for backward compatibility + dataset_type = getattr(cfg.data, "dataset_type", "volume") if dataset_type == "filename": # For filename-based datasets (PNG, JPG, etc.), use MONAI's LoadImaged @@ -455,8 +452,8 @@ def _build_augmentations(aug_cfg: AugmentationConfig, keys: list[str], do_2d: bo List of MONAI transforms """ transforms = [] - - # Get preset mode (default to "some" for backward compatibility) + + # Get preset mode preset = getattr(aug_cfg, "preset", "some") # Helper function to check if augmentation should be applied diff --git a/connectomics/lightning/callbacks.py b/connectomics/lightning/callbacks.py index e78ebc83..a2ac12a4 100644 --- a/connectomics/lightning/callbacks.py +++ b/connectomics/lightning/callbacks.py @@ -428,25 +428,7 @@ def create_callbacks(cfg) -> list: callbacks.append(vis_callback) # Model checkpoint callback - # Support both new unified config (training.checkpoint_*) and old separate config (checkpoint.*) - if hasattr(cfg, 'checkpoint') and cfg.checkpoint is not None: - # Old config style (backward compatibility) - monitor = getattr(cfg.checkpoint, 'monitor', 'val/loss') - default_filename = f'epoch={{epoch:03d}}-{monitor}={{{monitor}:.4f}}' - filename = getattr(cfg.checkpoint, 'filename', default_filename) - - checkpoint_callback = ModelCheckpoint( - monitor=monitor, - mode=getattr(cfg.checkpoint, 'mode', 'min'), - save_top_k=getattr(cfg.checkpoint, 'save_top_k', 3), - save_last=getattr(cfg.checkpoint, 'save_last', True), - dirpath=getattr(cfg.checkpoint, 'dirpath', 'checkpoints'), - filename=filename, - verbose=True - ) - callbacks.append(checkpoint_callback) - elif hasattr(cfg, 'monitor') and hasattr(cfg.monitor, 'checkpoint'): - # New unified config style (monitor.checkpoint.*) + if hasattr(cfg, 'monitor') and hasattr(cfg.monitor, 'checkpoint'): monitor = getattr(cfg.monitor.checkpoint, 'monitor', 'val/loss') filename = getattr(cfg.monitor.checkpoint, 'filename', None) if filename is None: @@ -465,19 +447,7 @@ def create_callbacks(cfg) -> list: callbacks.append(checkpoint_callback) # Early stopping callback - # Support both new unified config (training.early_stopping_*) and old separate config (early_stopping.*) - if hasattr(cfg, 'early_stopping') and cfg.early_stopping is not None and cfg.early_stopping.enabled: - # Old config style (backward compatibility) - early_stop_callback = EarlyStopping( - monitor=getattr(cfg.early_stopping, 'monitor', 'val/loss'), - patience=getattr(cfg.early_stopping, 'patience', 10), - mode=getattr(cfg.early_stopping, 'mode', 'min'), - min_delta=getattr(cfg.early_stopping, 'min_delta', 0.0), - verbose=True - ) - callbacks.append(early_stop_callback) - elif hasattr(cfg, 'monitor') and hasattr(cfg.monitor, 'early_stopping') and getattr(cfg.monitor.early_stopping, 'enabled', False): - # New unified config style (monitor.early_stopping.*) + if hasattr(cfg, 'monitor') and hasattr(cfg.monitor, 'early_stopping') and getattr(cfg.monitor.early_stopping, 'enabled', False): early_stop_callback = EarlyStopping( monitor=getattr(cfg.monitor.early_stopping, 'monitor', 'val/loss'), patience=getattr(cfg.monitor.early_stopping, 'patience', 10), diff --git a/connectomics/lightning/lit_model.py b/connectomics/lightning/lit_model.py index d4f27f5c..21fc8651 100644 --- a/connectomics/lightning/lit_model.py +++ b/connectomics/lightning/lit_model.py @@ -597,17 +597,13 @@ def _apply_postprocessing(self, data: np.ndarray) -> np.ndarray: data = np.stack(results, axis=0) print(f" DEBUG: _apply_postprocessing - after stacking, data shape: {data.shape}") - # Step 2: Apply scaling if configured (support both new and legacy names) + # Step 2: Apply scaling if configured intensity_scale = getattr(postprocessing, 'intensity_scale', None) - output_scale = getattr(postprocessing, 'output_scale', None) - scale = intensity_scale if intensity_scale is not None else output_scale - if scale is not None: - data = data * scale + if intensity_scale is not None: + data = data * intensity_scale - # Step 3: Apply dtype conversion if configured (support both new and legacy names) - intensity_dtype = getattr(postprocessing, 'intensity_dtype', None) - output_dtype = getattr(postprocessing, 'output_dtype', None) - target_dtype_str = intensity_dtype if intensity_dtype is not None else output_dtype + # Step 3: Apply dtype conversion if configured + target_dtype_str = getattr(postprocessing, 'intensity_dtype', None) if target_dtype_str is not None and target_dtype_str != 'float32': # Map string dtype to numpy dtype @@ -739,27 +735,6 @@ def _apply_decode_mode(self, data: np.ndarray) -> np.ndarray: decode_fn = decode_fn_map[fn_name] - # Backward compatibility: convert old parameter format to new tuple format - # for decode_binary_contour_distance_watershed - if fn_name == 'decode_binary_contour_distance_watershed': - if 'seed_threshold' in kwargs or 'foreground_threshold' in kwargs: - warnings.warn( - "Detected legacy parameters (seed_threshold, contour_threshold, foreground_threshold) " - "for decode_binary_contour_distance_watershed. Converting to new tuple format " - "(binary_threshold, contour_threshold, distance_threshold). " - "Please update your config files to use the new format.", - DeprecationWarning, - ) - # Convert old parameters to new tuple format - seed_thresh = kwargs.pop('seed_threshold', 0.9) - contour_thresh = kwargs.pop('contour_threshold', 0.8) - foreground_thresh = kwargs.pop('foreground_threshold', 0.85) - - # Map old parameters to new tuple format - kwargs['binary_threshold'] = (seed_thresh, foreground_thresh) - kwargs['contour_threshold'] = (contour_thresh, 1.1) - kwargs['distance_threshold'] = (0.5, -0.5) - try: # Apply decoding function sample = decode_fn(sample, **kwargs) diff --git a/connectomics/models/loss/regularization.py b/connectomics/models/loss/regularization.py index 9d6101d3..5a68060d 100644 --- a/connectomics/models/loss/regularization.py +++ b/connectomics/models/loss/regularization.py @@ -294,14 +294,6 @@ def forward(self, pred: torch.Tensor) -> torch.Tensor: return loss.mean() -# Aliases for backward compatibility (if needed in configs) -BinaryReg = BinaryRegularization -FgDTConsistency = ForegroundDistanceConsistency -ContourDTConsistency = ContourDistanceConsistency -FgContourConsistency = ForegroundContourConsistency -NonoverlapReg = NonOverlapRegularization - - __all__ = [ 'BinaryRegularization', 'ForegroundDistanceConsistency', diff --git a/connectomics/utils/visualizer.py b/connectomics/utils/visualizer.py index c3a0895b..228797f4 100755 --- a/connectomics/utils/visualizer.py +++ b/connectomics/utils/visualizer.py @@ -590,9 +590,3 @@ def _should_visualize(self, trainer, batch_idx: int) -> bool: log_every_n_steps = getattr(self.cfg.optimization, "vis_every_n_steps", 100) return trainer.global_step % log_every_n_steps == 0 and batch_idx == 0 - - -# Legacy compatibility -def create_visualizer(cfg, **kwargs): - """Factory function for creating visualizer (backward compatible).""" - return Visualizer(cfg, **kwargs) From 741d836662e82d5c614a6b0bfdc7e5075b505ea6 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Nov 2025 17:10:09 +0000 Subject: [PATCH 3/3] Remove legacy config fields and their fallback logic Completely removes deprecated configuration fields instead of just comments: **Removed Config Fields:** 1. `DataConfig.test_transpose` - Use `inference.data.test_transpose` instead 2. `TestTimeAugmentationConfig.act` - Use `channel_activations` instead **Removed Fallback Logic:** 1. build.py: Remove cfg.data.test_transpose fallback (lines 262-263) - Now only uses cfg.inference.data.test_transpose 2. lit_model.py: Remove single activation fallback (lines 331-348) - Now requires channel_activations for TTA **Impact:** - 3 files modified - 24 lines removed - Breaking change: Old configs using deprecated fields will no longer work - Users must update to modern config structure --- connectomics/config/hydra_config.py | 2 -- connectomics/data/augment/build.py | 6 ++---- connectomics/lightning/lit_model.py | 18 ------------------ 3 files changed, 2 insertions(+), 24 deletions(-) diff --git a/connectomics/config/hydra_config.py b/connectomics/config/hydra_config.py index 31c4edee..0e76e9cb 100644 --- a/connectomics/config/hydra_config.py +++ b/connectomics/config/hydra_config.py @@ -392,7 +392,6 @@ class DataConfig: default_factory=list ) # Axis permutation for training data (e.g., [2,1,0] for xyz->zyx) val_transpose: List[int] = field(default_factory=list) # Axis permutation for validation data - test_transpose: List[int] = field(default_factory=list) # Axis permutation for test data # Dataset statistics (for auto-planning) target_spacing: Optional[List[float]] = None # Target voxel spacing [z, y, x] in mm @@ -866,7 +865,6 @@ class TestTimeAugmentationConfig: flip_axes: Any = ( None # TTA flip strategy: "all" (8 flips), null (no aug), or list like [[0], [1], [2]] ) - act: Optional[str] = None # Single activation for all channels: 'softmax', 'sigmoid', 'tanh', None channel_activations: Optional[List[Any]] = ( None # Per-channel activations: [[start_ch, end_ch, 'activation'], ...] e.g., [[0, 2, 'softmax'], [2, 3, 'sigmoid'], [3, 4, 'tanh']] ) diff --git a/connectomics/data/augment/build.py b/connectomics/data/augment/build.py index 15aca44b..a4325d73 100644 --- a/connectomics/data/augment/build.py +++ b/connectomics/data/augment/build.py @@ -257,17 +257,15 @@ def _build_eval_transforms_impl( if mode == "val": transpose_axes = cfg.data.val_transpose if cfg.data.val_transpose else [] else: # mode == "test" - # Check both data.test_transpose and inference.data.test_transpose + # Use inference.data.test_transpose transpose_axes = [] - if cfg.data.test_transpose: - transpose_axes = cfg.data.test_transpose if ( hasattr(cfg, "inference") and hasattr(cfg.inference, "data") and hasattr(cfg.inference.data, "test_transpose") and cfg.inference.data.test_transpose ): - transpose_axes = cfg.inference.data.test_transpose # inference takes precedence + transpose_axes = cfg.inference.data.test_transpose transforms.append( LoadVolumed(keys=keys, transpose_axes=transpose_axes if transpose_axes else None) diff --git a/connectomics/lightning/lit_model.py b/connectomics/lightning/lit_model.py index 21fc8651..a2fcfb67 100644 --- a/connectomics/lightning/lit_model.py +++ b/connectomics/lightning/lit_model.py @@ -328,24 +328,6 @@ def _apply_tta_preprocessing(self, tensor: torch.Tensor) -> torch.Tensor: # Concatenate all channels back together tensor = torch.cat(activated_channels, dim=1) - else: - # Fall back to single activation for all channels (old approach) - tta_act = getattr(self.cfg.inference.test_time_augmentation, 'act', None) - if tta_act is None: - tta_act = getattr(self.cfg.inference, 'output_act', None) - - # Apply activation function - if tta_act == 'softmax': - tensor = torch.softmax(tensor, dim=1) - elif tta_act == 'sigmoid': - tensor = torch.sigmoid(tensor) - elif tta_act == 'tanh': - tensor = torch.tanh(tensor) - elif tta_act is not None and tta_act.lower() != 'none': - warnings.warn( - f"Unknown TTA activation function '{tta_act}'. Supported: 'softmax', 'sigmoid', 'tanh', None", - UserWarning, - ) # Get TTA-specific channel selection or fall back to output_channel tta_channel = getattr(self.cfg.inference.test_time_augmentation, 'select_channel', None)