Classification : retaining prediction and fetching data from s3 for model evaluation #359
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
nishika26
changed the title
nishika26
changed the title
| "test_data_s3_url", sqlmodel.sql.sqltypes.AutoString(), nullable=False | ||
| ), | ||
| ) | ||
| op.add_column( | ||
| "model_evaluation", | ||
| sa.Column( | ||
| "prediction_data_s3_url", sqlmodel.sql.sqltypes.AutoString(), nullable=True |
There was a problem hiding this comment.
keep this consistent with other migrations, somewhere i saw it was just data_url and here it is data_s3_url
There was a problem hiding this comment.
Yes I have turned all the "data_url" to "data_s3_url" , and I have made sure that this is consistent
| client = get_openai_client( | ||
| session, current_user.organization_id, current_user.project_id | ||
| ) | ||
| ) # keeping this here for checking if the user's validated OpenAI key is present or not, |
There was a problem hiding this comment.
didn't understand this part
There was a problem hiding this comment.
The client that actually carries out the OpenAI functions will be initialized inside the background task. The client being created in the router isn’t passed to the background task, because it’s generally not a good practice to pass objects into background tasks, and can cause same error as when passing router session to bg task. However, we still want to validate upfront that the user invoking this endpoint has a valid OpenAI key stored in our database. If we leave that check only to the background task, the user wouldn’t immediately know if their key is invalid or not present. That’s why the client is initialized in the router for validation purposes, while the actual client used to perform operations is re-initialized inside the background task.
* Classification: db models and migration script (#305) * db models and migration script * Classification: Fine tuning Initiation and retrieve endpoint (#315) * Fine-tuning core, initiation, and retrieval * seperate session for bg task, and formating fixes * fixing alembic revision * Classification : Model evaluation of fine tuned models (#326) * Model evaluation of fine tuned models * fixing alembic revision * alembic revision fix * Classification : train and test data to s3 (#343) * alembic file for adding and removing columns * train and test s3 url column * updating alembic revision * formatting fix * Classification : retaining prediction and fetching data from s3 for model evaluation (#359) * adding new columns to model eval table * test data and prediction data s3 url changes * single migration file * status enum columns * document seeding * Classification : small fixes and storage related changes (#365) * first commit covering all * changing model name to fine tuned model in model eval * error handling in get cloud storage and document not found error handling * fixing alembic revision * uv lock * new uv lock file * updated uv lock file * coderabbit suggestions and removing unused imports * changes in uv lock file * making csv a supported file format, changing uv lock and pyproject toml
2 participants
Summary
Target issue is #344
Checklist
Before submitting a pull request, please ensure that you mark these task.
fastapi run --reload app/main.pyordocker compose upin the repository root and test.Notes
Database Migration and Model Schema Changes -
app/models/model_evaluation.pyDropped testing_file_id column from model_evaluation.
Added:
test_data_s3_url → S3 location for evaluation dataset .
prediction_data_s3_url → S3 location for generated predictions.
THIS IS AN EXAMPLE OF WHAT THE DATA INSIDE THE PREDICTION DATA S3 URL WOULD BE - local_copy_6.csv
ModelEvaluation updated to reflect new S3-based fields.
ModelEvaluationUpdate and ModelEvaluationPublic extended with prediction_data_s3_url.
Model Evaluator Enhancements -
app/core/finetune/evaluation.pyModelEvaluator now:
Loads test data CSVs from S3 instead of OpenAI NDJSON files.
Extracts prompts and labels directly from CSV (query|question|message + label).
Saves predictions to a CSV, uploads them to S3, and records the resulting URL.
Computes evaluation metrics (currently MCC score) based on streamed CSV data from S3.
API & CRUD Adjustments -
app/api/routes/model_evaluation.py&app/crud/model_evaluation.pyAPI routes now pass test_data_s3_url instead of testing_file_id.
Evaluation run stores both:
score (evaluation metrics).
prediction_data_s3_url (uploaded predictions CSV).
Data Preprocessor Updates -
app/core/finetune/preprocessing.pyupload_csv_to_s3refactored into a @staticmethod for reusability in the ModelEvaluator class.Tests -
app/tests/utils/test_data.py&app/tests/crud/test_model_evaluation.pyUpdated unit tests and fixtures to use test_data_s3_url instead of testing_file_id.
Ensured CRUD and utils tests cover the new fields.