Skip to content

Commit 4eea486

Browse files
committed
fix(models): remove redundant preprocessing and improve logging
1 parent 9f9fcb0 commit 4eea486

3 files changed

Lines changed: 20 additions & 21 deletions

File tree

src/main/java/sentiment/models/ModelLoader.java

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package sentiment.models;
22

3+
import org.slf4j.Logger;
4+
import org.slf4j.LoggerFactory;
35
import sentiment.preprocessing.TextPreprocessor;
46
import sentiment.preprocessing.WekaInstancesConverter;
57
import sentiment.training.TrainingMetadata;
@@ -18,6 +20,8 @@
1820
*/
1921
public class ModelLoader {
2022

23+
private static final Logger logger = LoggerFactory.getLogger(ModelLoader.class);
24+
2125
/**
2226
* Load model with metadata validation.
2327
* Throws exception if metadata file is missing (non-negotiable requirement).
@@ -43,23 +47,22 @@ public static SentimentClassifier loadWithMetadata(String modelPath)
4347

4448
// Load metadata first
4549
TrainingMetadata metadata = TrainingMetadata.load(metadataPath);
46-
System.out.printf("✓ Loaded metadata: %s, trained on %s, accuracy=%.3f%n",
50+
logger.info("Loaded metadata: {}, trained on {}, accuracy={}",
4751
metadata.getModelId(),
4852
metadata.getDataset().datasetName,
49-
metadata.getMetrics().testAccuracy);
53+
String.format("%.3f", metadata.getMetrics().testAccuracy));
5054

5155
// Validate metadata points to correct model file
5256
if (metadata.getModelFile() != null &&
5357
!metadata.getModelFile().equals(path.getFileName().toString())) {
54-
System.err.printf("⚠ Metadata model_file mismatch: expected %s, got %s%n",
58+
logger.warn("Metadata model_file mismatch: expected {}, got {}",
5559
metadata.getModelFile(), path.getFileName());
5660
}
5761

5862
// Load the actual model based on algorithm type
5963
SentimentClassifier classifier = loadModelByType(path, metadata);
6064

61-
System.out.printf("✓ Loaded model: %s from %s%n",
62-
path.getFileName(), path.getParent());
65+
logger.info("Loaded model: {} from {}", path.getFileName(), path.getParent());
6366

6467
return classifier;
6568
}
@@ -141,7 +144,7 @@ public static java.util.Map<String, SentimentClassifier> loadAllForAlgorithm(Str
141144
Path algoDir = Paths.get("models", algorithm);
142145

143146
if (!Files.exists(algoDir)) {
144-
System.err.println("⚠ Model directory not found: " + algoDir);
147+
logger.warn("Model directory not found: {}", algoDir);
145148
return models;
146149
}
147150

@@ -157,10 +160,10 @@ public static java.util.Map<String, SentimentClassifier> loadAllForAlgorithm(Str
157160
SentimentClassifier classifier = loadWithMetadata(modelPath.toString());
158161
models.put(domain, classifier);
159162

160-
System.out.printf("✓ Loaded %s model trained on %s%n", algorithm, domain);
163+
logger.info("Loaded {} model trained on {}", algorithm, domain);
161164

162165
} catch (Exception e) {
163-
System.err.println("✗ Failed to load " + modelPath + ": " + e.getMessage());
166+
logger.error("Failed to load {}: {}", modelPath, e.getMessage());
164167
}
165168
});
166169
}

src/main/java/sentiment/models/SVMClassifier.java

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -130,27 +130,22 @@ protected ClassifierEvaluationResult doTrain(List<Dataset> rawDatasets) throws E
130130

131131
logger.info("Training SVM on {} raw datasets with full pipeline", rawDatasets.size());
132132

133-
// Step 1: Fit preprocessing pipeline
134-
logger.info("Step 1/3: Fitting preprocessing pipeline");
135-
preprocessor.fit(rawDatasets);
136-
logger.info("Preprocessor fitted. Vocabulary: {}",
137-
preprocessor.getPipelineState().vocabularySize);
138-
139-
// Step 2: Fit feature extraction (converter) and get Instances
140-
logger.info("Step 2/3: Fitting feature extraction");
133+
// Step 1: Fit vectorization pipeline (preprocessing + TF-IDF)
134+
// Note: converter.fit() internally calls preprocessor.fit()
135+
logger.info("Step 1/2: Fitting vectorization pipeline");
141136
Instances trainingInstances = converter.fit(rawDatasets);
142-
logger.info("Converter fitted. Features: {}, Vocabulary: {}",
137+
logger.info("Vectorization complete. Features: {}, Vocabulary: {}",
143138
trainingInstances.numAttributes() - 1,
144139
converter.getVocabulary().size());
145140

146-
// Step 3: Train SVM on converted Instances
147-
logger.info("Step 3/3: Training SVM classifier");
141+
// Step 2: Train SVM on converted Instances
142+
logger.info("Step 2/2: Training SVM classifier");
148143
validateWekaTrainingData(trainingInstances);
149144
configureSMOForTraining(trainingInstances);
150145
performAlgorithmSpecificTraining(trainingInstances);
151146
finalizeTraining(trainingInstances);
152147

153-
// Step 4: Validate pipeline consistency (CRITICAL)
148+
// Validate pipeline consistency
154149
validatePipelineConsistency();
155150

156151
logger.info("SVM training complete. Pipeline ready for inference.");

src/test/java/sentiment/models/SVMClassifierTest.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,8 @@ void testTrain_FitsPipelineComponents() throws Exception {
148148
classifier.train(trainingData);
149149

150150
// Assert
151-
verify(mockPreprocessor, times(1)).fit(trainingData);
151+
// WekaInstancesConverter now owns preprocessor fitting, so we only verify converter.fit()
152+
// The preprocessor.fit() happens internally within converter.fit()
152153
verify(mockConverter, times(1)).fit(trainingData);
153154
assertTrue(classifier.isTrained(), "Classifier should be trained");
154155
}

0 commit comments

Comments
 (0)