You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Description of Refactoring/Improvement
Update get_loss_metrics_monitor_and_outputs() and related training setup code to use the new TrainingDataAdapter interface (from Issue #199) instead of directly accessing DataConfigHandler internals. This completes the decoupling of training logic from dataset implementation details.
Goals and Objectives
Eliminate hardcoded dataset field access from training logic
Simplify get_loss_metrics_monitor_and_outputs() signature and implementation
Enable clean support for both simulation and real data workflows
Improve code readability and maintainability
Current Code Behavior get_loss_metrics_monitor_and_outputs() currently:
Simplification: Remove ~15 lines of conditional dataset access logic
Clarity: Function dependencies explicit via type hints
Testability: Can test with mock adapters (no need for full DataConfigHandler setup)
Flexibility: Switching data sources now requires only changing adapter, not training code
Type Safety: IDE autocomplete and type checking work properly
Dependencies
Requires: Issue #199 (TrainingDataAdapter implementation) to be completed first
Affects:
Main training script(s) that call get_loss_metrics_monitor_and_outputs()
Any test code that uses this function
Documentation/examples showing training setup
Testing Plan
Unit tests for refactored function:
Test with mock TrainingDataAdapter (verify it calls correct methods)
Test both masked and unmasked loss configurations
Verify correct loss/metrics objects returned
Integration tests:
Test with real PreSplitDataAdapter (simulation workflow)
Test with real SplitOnLoadDataAdapter (real data workflow)
Verify training loop works end-to-end with both adapter types
Regression tests:
Ensure existing simulation training produces identical results
Additional Context
This refactoring is the second step in modernizing the data handling architecture. Issue #199 introduces the adapter abstraction, while this issue migrates the training code to actually use it. Breaking change consideration: The function signature changes, so this is technically a breaking change.
Impact Assessment
Medium impact, high value:
Breaking change to get_loss_metrics_monitor_and_outputs() signature
All code calling this function needs updating (estimate: 3-5 call sites)
Significantly improves code quality and maintainability
Unblocks future work on real data training pipeline
Sets precedent for adapter pattern usage elsewhere in codebase
Migration effort: ~2-4 hours for implementation + testing
Description of Refactoring/Improvement
Update
get_loss_metrics_monitor_and_outputs()and related training setup code to use the newTrainingDataAdapterinterface (from Issue #199) instead of directly accessingDataConfigHandlerinternals. This completes the decoupling of training logic from dataset implementation details.Goals and Objectives
get_loss_metrics_monitor_and_outputs()signature and implementationCurrent Code Behavior
get_loss_metrics_monitor_and_outputs()currently:data_conf(entire DataConfigHandler object)data_conf.training_data.datasetstructure"noisy_stars","stars","masks"Proposed Changes
Expected Benefits
Dependencies
Requires: Issue #199 (TrainingDataAdapter implementation) to be completed first
Affects:
Testing Plan
TrainingDataAdapter(verify it calls correct methods)PreSplitDataAdapter(simulation workflow)SplitOnLoadDataAdapter(real data workflow)Additional Context
This refactoring is the second step in modernizing the data handling architecture. Issue #199 introduces the adapter abstraction, while this issue migrates the training code to actually use it.
Breaking change consideration: The function signature changes, so this is technically a breaking change.
Impact Assessment
Medium impact, high value:
get_loss_metrics_monitor_and_outputs()signatureMigration effort: ~2-4 hours for implementation + testing
Next Steps
TrainingDataAdapterabstraction for simulation data #199 is merged and adapters are availableget_loss_metrics_monitor_and_outputs()signature and implementationCHANGELOGwith migration guide if breaking changeDataConfigHandlerto remove now-unused attributes (separate issue)Thank you for starting this request to refactor or improve the code. We will review it and collaborate to enhance the codebase together! 🛠️