SIENTIAPDE-1241: Refactor: Remove redundant logging and fix duplicate logs
This commit removes redundant logging statements from training and experiment tracking activities, preventing duplicate log entries. It also introduces a logger helper to disable log propagation, further addressing the duplicate logs issue. Additionally, the Makefile, run_coverage.sh, setup_port_forwards.sh, and simulator/Dockerfile files were removed as they are no longer needed.
This commit is contained in:
@@ -89,9 +89,6 @@ class ExperimentTracking(Postgres):
|
||||
notification_handler=notification_handler,
|
||||
)
|
||||
|
||||
self.logger = logger
|
||||
self.notification_handler = notification_handler
|
||||
|
||||
def __del__(self):
|
||||
"""
|
||||
Destructor to safely handle cleanup during garbage collection.
|
||||
@@ -160,10 +157,6 @@ class ExperimentTracking(Postgres):
|
||||
run_name = input_data.get('run_name')
|
||||
|
||||
try:
|
||||
self.info(
|
||||
f'Updating experiment run {experiment_run_id} with status: {status}', metadata
|
||||
)
|
||||
|
||||
query_params: dict[str, Any]
|
||||
|
||||
if update_type == UpdateType.STATUS:
|
||||
@@ -249,6 +242,4 @@ class ExperimentTracking(Postgres):
|
||||
level=NotificationLevel.ERROR,
|
||||
attachment_content=trace,
|
||||
)
|
||||
|
||||
self.error(trace, metadata=metadata)
|
||||
raise RuntimeError(error_msg) from e
|
||||
|
||||
@@ -80,7 +80,6 @@ class Training(BaseActivity):
|
||||
metadata = input_data.get('metadata', {})
|
||||
|
||||
try:
|
||||
self.info('Validating training parameters', metadata)
|
||||
train_params = TrainModelParams.from_dict(input_data)
|
||||
train_params.validate_business_rules()
|
||||
|
||||
@@ -104,8 +103,6 @@ class Training(BaseActivity):
|
||||
level=NotificationLevel.ERROR,
|
||||
attachment_content=trace,
|
||||
)
|
||||
|
||||
self.error(trace, metadata=metadata)
|
||||
raise
|
||||
|
||||
@activity.defn(name='train_model')
|
||||
@@ -177,8 +174,6 @@ class Training(BaseActivity):
|
||||
attachment_content=trace,
|
||||
)
|
||||
|
||||
self.error(trace, metadata=metadata)
|
||||
|
||||
raise ModelTrainingError(
|
||||
model_trained=model_trained,
|
||||
model_saved=model_saved,
|
||||
@@ -223,6 +218,4 @@ class Training(BaseActivity):
|
||||
level=NotificationLevel.ERROR,
|
||||
attachment_content=trace,
|
||||
)
|
||||
|
||||
self.error(trace, metadata=metadata)
|
||||
raise
|
||||
|
||||
27
model_manager/utils/logger_helper.py
Normal file
27
model_manager/utils/logger_helper.py
Normal file
@@ -0,0 +1,27 @@
|
||||
"""
|
||||
Logger helper to prevent duplicate logs caused by propagation.
|
||||
|
||||
This module provides a wrapper around sientia_do Logger to disable
|
||||
log propagation and prevent duplicate log entries in the Model Manager.
|
||||
"""
|
||||
|
||||
from sientia_do.observability.logger import Logger as SientiaLogger
|
||||
|
||||
|
||||
def get_logger(name: str) -> SientiaLogger:
|
||||
"""
|
||||
Create a Logger instance with propagation disabled.
|
||||
|
||||
This prevents duplicate logs caused by hierarchical propagation
|
||||
in Python's logging system.
|
||||
|
||||
Args:
|
||||
name: Logger name (typically __name__ of the calling module).
|
||||
|
||||
Returns:
|
||||
Logger: Configured logger instance with propagation disabled.
|
||||
"""
|
||||
logger = SientiaLogger(name)
|
||||
# Disable propagation to prevent duplicate logs
|
||||
logger.base_logger.propagate = False
|
||||
return logger
|
||||
@@ -55,25 +55,14 @@ class ModelRepository:
|
||||
Exception: If model saving fails (after sending notification)
|
||||
"""
|
||||
experiment_name = train_result.params.experiment_name
|
||||
self.logger.info(f'Starting model save for experiment: {experiment_name}')
|
||||
|
||||
# Step 1: Generate next run name
|
||||
self.logger.info('Generating run name')
|
||||
train_result.run_name = self._get_next_run_name(experiment_name)
|
||||
self.logger.info(f'Generated run name: {train_result.run_name}')
|
||||
|
||||
# Step 2: Generate artifacts (reports, CSV files)
|
||||
self.logger.info('Generating artifacts')
|
||||
train_result = self._generate_artifacts(train_result)
|
||||
self.logger.info('Artifacts generated successfully')
|
||||
|
||||
# Step 3: Save run to MLflow
|
||||
self.logger.info('Saving run to MLflow')
|
||||
self._save_run(train_result)
|
||||
|
||||
self.logger.info(
|
||||
f'Model saved successfully - Run: {train_result.run_name}, '
|
||||
f'Experiment: {experiment_name}'
|
||||
f'Model saved successfully - experiment run id: {train_result.params.experiment_run_id}, '
|
||||
f'experiment name: {experiment_name}, '
|
||||
f'run name: {train_result.run_name}'
|
||||
)
|
||||
|
||||
return train_result
|
||||
@@ -93,8 +82,6 @@ class ModelRepository:
|
||||
self.logger.info('No run directory specified, skipping cleanup')
|
||||
return
|
||||
|
||||
self.logger.info(f'Cleaning up run directory: {run_dir}')
|
||||
|
||||
if os.path.exists(run_dir):
|
||||
shutil.rmtree(run_dir)
|
||||
self.logger.info(f'Run directory deleted successfully: {run_dir}')
|
||||
|
||||
@@ -102,7 +102,6 @@ class StorageRepository:
|
||||
Raises:
|
||||
OSError: If the download fails (network, permissions, missing key, etc.).
|
||||
"""
|
||||
self.logger.info(f'Fetching file from MinIO: {bucket_name}/{file_name}')
|
||||
response = self.minio_client.get_object(Bucket=bucket_name, Key=file_name)
|
||||
|
||||
with response['Body'] as body:
|
||||
@@ -124,6 +123,5 @@ class StorageRepository:
|
||||
bucket_name: Bucket that contains the object.
|
||||
file_name: Object key to delete.
|
||||
"""
|
||||
self.logger.info(f'Deleting file from MinIO: {bucket_name}/{file_name}')
|
||||
self.minio_client.delete_object(Bucket=bucket_name, Key=file_name)
|
||||
self.logger.info(f'File deleted successfully: {bucket_name}/{file_name}')
|
||||
|
||||
@@ -66,9 +66,7 @@ class TrainingRepository:
|
||||
ValueError: If transformed data is empty
|
||||
Exception: If data loading, preprocessing, or training fails
|
||||
"""
|
||||
self.logger.info('Loading data from BytesIO file')
|
||||
data = load_data(uploaded_file, params.line_separator, params.decimal_separator)
|
||||
self.logger.info('Initializing and fitting data preprocessor')
|
||||
process_data = self._init_data_preprocessor(params)
|
||||
process_data.fit(data)
|
||||
data_view = process_data.transform(data)
|
||||
@@ -76,7 +74,6 @@ class TrainingRepository:
|
||||
if len(data_view) <= 0:
|
||||
raise ValueError('Data view is empty after transformation')
|
||||
|
||||
self.logger.info('Splitting data into train/test sets')
|
||||
x_train, x_test, y_train, y_test = split_train_test(
|
||||
data_view[params.variable_columns],
|
||||
data_view[params.target_variable],
|
||||
@@ -85,17 +82,18 @@ class TrainingRepository:
|
||||
random_state=42,
|
||||
)
|
||||
|
||||
self.logger.info('Preparing training data')
|
||||
data_train = pd.concat([x_train, y_train], axis=1)
|
||||
scaler_dict = self._init_scaler_dict(process_data, params)
|
||||
|
||||
self.logger.info('Training linear regression model')
|
||||
regr = LinearRegressionModel(
|
||||
target_variable=params.target_variable,
|
||||
variable_columns=params.variable_columns,
|
||||
)
|
||||
|
||||
regr.fit(data_train)
|
||||
self.logger.info(
|
||||
f'Model trained successfully - experiment run id: {params.experiment_run_id}'
|
||||
)
|
||||
|
||||
return TrainModelResult(
|
||||
params=params,
|
||||
@@ -128,12 +126,10 @@ class TrainingRepository:
|
||||
TrainModelResult: Updated result with predictions, denormalized data,
|
||||
and metrics (mse_val, mae_val, r2_val)
|
||||
"""
|
||||
self.logger.info('Making predictions on test set')
|
||||
y_pred_array = tmr.regr.predict(tmr.x_test)
|
||||
|
||||
if params.use_scaler:
|
||||
scaler = tmr.process_data.get_scaler()
|
||||
self.logger.info('Denormalizing features')
|
||||
|
||||
# If using custom scaler with denormalize_* helpers
|
||||
if hasattr(scaler, 'denormalize_single_input'):
|
||||
@@ -141,7 +137,6 @@ class TrainingRepository:
|
||||
tmr.x_train[col] = scaler.denormalize_single_input(tmr.x_train[col], col)
|
||||
tmr.x_test[col] = scaler.denormalize_single_input(tmr.x_test[col], col)
|
||||
|
||||
self.logger.info('Denormalizing target variable')
|
||||
tmr.y_train = scaler.denormalize_single_input(tmr.y_train, params.target_variable)
|
||||
tmr.y_test = scaler.denormalize_single_input(tmr.y_test, params.target_variable)
|
||||
y_pred_array = scaler.denormalize_predictions(y_pred_array, params.target_variable)
|
||||
@@ -158,18 +153,15 @@ class TrainingRepository:
|
||||
tmr.x_test[feature_cols] = scaler.inverse_transform(x_test_features)
|
||||
# Target was not scaled with StandardScaler in preprocessing; leave y as-is
|
||||
|
||||
self.logger.info('Adding index to predictions')
|
||||
tmr.y_pred = pd.Series(y_pred_array, index=tmr.y_test.index)
|
||||
tmr.y_pred.name = f'{params.target_variable}_pred'
|
||||
|
||||
self.logger.info('Reordering all data by index')
|
||||
tmr.x_train = tmr.x_train.sort_index()
|
||||
tmr.x_test = tmr.x_test.sort_index()
|
||||
tmr.y_train = tmr.y_train.sort_index()
|
||||
tmr.y_test = tmr.y_test.sort_index()
|
||||
tmr.y_pred = tmr.y_pred.sort_index()
|
||||
|
||||
self.logger.info('Calculating evaluation metrics')
|
||||
assert tmr.y_pred is not None, 'y_pred should be set at this point'
|
||||
|
||||
tmr.mse_val = round(
|
||||
@@ -183,6 +175,9 @@ class TrainingRepository:
|
||||
)
|
||||
|
||||
tmr.r2_val = round(r2(tmr.y_test.astype(np.float64), tmr.y_pred.astype(np.float64)), 2)
|
||||
self.logger.info(
|
||||
f'Model metrics calculated successfully - experiment run id: {params.experiment_run_id}'
|
||||
)
|
||||
return tmr
|
||||
|
||||
def _init_scaler_dict(self, process_data: DataPreprocessor, params: TrainModelParams) -> dict:
|
||||
|
||||
@@ -33,7 +33,6 @@ with workflow.unsafe.imports_passed_through():
|
||||
|
||||
from prometheus_client import start_http_server
|
||||
from sientia_do.notifications.handlers import CoreNotificationHandler as NotificationHandler
|
||||
from sientia_do.observability.logger import get_logger
|
||||
|
||||
from model_manager import metrics
|
||||
from model_manager.activities.activities import Activities
|
||||
@@ -43,6 +42,7 @@ with workflow.unsafe.imports_passed_through():
|
||||
build_mongodb_config,
|
||||
build_postgres_config,
|
||||
)
|
||||
from model_manager.utils.logger_helper import get_logger
|
||||
from model_manager.workflows.train_model import TrainModel
|
||||
|
||||
POD_ID = os.getenv('POD_ID')
|
||||
@@ -73,7 +73,7 @@ async def main():
|
||||
|
||||
metadata = {
|
||||
'pod_id': POD_ID,
|
||||
'workflow_name': '-',
|
||||
'workflow_name': 'train_model',
|
||||
}
|
||||
|
||||
logger.custom_info(f'Starting Worker with POD_ID: {POD_ID}', metadata)
|
||||
|
||||
@@ -53,6 +53,8 @@ with workflow.unsafe.imports_passed_through():
|
||||
maximum_attempts=5,
|
||||
)
|
||||
|
||||
POD_ID = os.getenv('POD_ID')
|
||||
|
||||
|
||||
@workflow.defn(name='train_model')
|
||||
class TrainModel:
|
||||
@@ -98,6 +100,7 @@ class TrainModel:
|
||||
|
||||
metadata = {
|
||||
'metadata': {
|
||||
'pod_id': POD_ID,
|
||||
'experiment_run_id': experiment_run_id,
|
||||
'workflow_name': 'train_model',
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user