Feature/holdout evaluation#19
Merged
Merged
Conversation
The evaluation "holdout" was an independent random sample of the same gold table (orderBy(rand(7)).limit) with a different seed than training. Different seeds do not guarantee disjoint rows, so training data leaked into evaluation and the ROC AUC was optimistic. Implement a proper hold-out following Databricks best practice: - Feature engineering materialises a deterministic train/validation/test split as a Delta table (transactions_split), assigning each transaction by pmod(xxhash64(transaction_id), 100): ~70/15/15. Same id -> same split on every run and in every notebook, so the split is reproducible, leakage-free, versioned. - Training reads only the train + validation rows (join on the split table), disjoint from the test holdout. - Evaluation reads only the test split, ordered deterministically so the candidate and champion are scored on the identical rows. Notebook notes cover the production hardening: hash the entity id to avoid entity leakage, a time-based holdout for temporal fraud data, and point-in-time feature lookups to prevent feature leakage.
…ld in training Make the hold-out representative and leakage-free for an imbalanced, multi-row-per -customer problem: - Split is now GROUPED BY CUSTOMER and STRATIFIED BY FRAUD. Feature engineering assigns whole clients to train/validation/test (never a single transaction), so the same customer can never appear in both train and test (no entity leakage). Assignment uses percent_rank over a hash of client_id WITHIN each fraud/non-fraud stratum, so the fraud rate is identical across splits (representative) and the result is deterministic and versioned. - Training replaces the single train_test_split with StratifiedGroupKFold (n_splits=5, grouped by client_id): each fold stratifies by the rare label and keeps a client in one fold, so no customer is in both the fit and validation of a fold. Metrics are averaged across folds (honest estimate); the registered model is then fit on all train+validation rows. client_id is kept as the CV group and the pyfunc wrapper selects the feature columns so it is not used as a model feature.
The batch monitor compared each window only against the previous windows, so "drift" meant window-to-window wobble rather than movement away from what the model was validated on. Databricks recommends the model's train/validation data as the reference distribution for an inference monitor. - batch_inference now scores the held-out validation split with the champion and writes it, in the exact schema of fraud_predictions, to fraud_predictions_baseline (overwritten each run so it tracks the champion). - batch_monitor sets baseline_table_name to that table, so drift is measured against the validation distribution. Commented inline to explain why.
… X_test/y_test The CV rewrite removed the single X_test/y_test split, but the confusion-matrix cell still referenced them (NameError at run time). Add a grouped demo fold (first StratifiedGroupKFold split) and plot naive vs a balanced demo model on that held-out fold. Verified end to end on dev: feature engineering, grouped k-fold training, evaluation gate on the test split, and the validation-split baseline all run.
BenConstable9
force-pushed
the
feature/holdout-evaluation
branch
from
July 16, 2026 09:09
f58e3fa to
e19f732
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR strengthens model evaluation and monitoring by introducing a deterministic, leakage-resistant train/validation/test split (grouped by customer and stratified by fraud) and updating training/evaluation workflows to consistently use the persisted holdout.
Changes:
- Materialize a deterministic, customer-grouped, label-stratified 70/15/15 split table and use it to keep training data disjoint from the
testholdout. - Update training to use stratified group k-fold cross-validation (grouped by
client_id) and register a final model trained on balanced train+validation data. - Update evaluation to score registered models on the persisted
testsplit, and add a validation-split baseline table for monitoring drift comparisons.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/training/notebooks/training.py | Train only on train+validation rows and estimate generalization via StratifiedGroupKFold; ensure serving excludes non-feature columns. |
| src/feature_engineering/notebooks/build_features.py | Write a deterministic, grouped/stratified split table (transactions_split) for reproducible holdout evaluation. |
| src/deployment/model_deployment/notebooks/evaluation.py | Evaluate candidate/champion on the persisted test split (deterministic) to avoid leakage. |
| src/deployment/batch_inference/notebooks/batch_inference.py | Write a validation-split-scored baseline table for drift monitoring comparisons. |
| solution/training/notebooks/training.py | Reference implementation of the updated grouped CV + holdout-disjoint training and registration flow. |
| solution/feature_engineering/notebooks/build_features.py | Reference implementation of the deterministic grouped/stratified split materialization. |
| solution/deployment/model_deployment/notebooks/evaluation.py | Reference implementation of scoring on the persisted test split. |
| solution/deployment/batch_inference/notebooks/batch_inference.py | Reference implementation of writing the validation baseline table for monitoring. |
| resources/monitoring-resource.yml | Configure the batch monitor to use the validation baseline table as its drift reference. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…ld balancing Replace the percent_rank grouped/stratified split with a pure hash bucket (pmod(xxhash64(client_id), 100): <70 train, <85 validation, else test). The per-customer grouping and approximate fraud stratification are preserved, but the cut is population-stable so a client never flips between splits as new data arrives. Guard balance() so a fold with no fraud (or no non-fraud) rows falls back to the fold unchanged, and never samples more non-fraud rows than exist, preventing an empty training set that would fail RandomForestClassifier.fit. Also mirror the earlier src-only double-spark-job fix into the batch_inference solution so regen stays consistent.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.