add logistic regression baseline PS and an azure job to run it - #197
add logistic regression baseline PS and an azure job to run it#197kirilklein wants to merge 2 commits into
Conversation
Reviewers asked for a conventional propensity score model to compare against the transformer PS. The CatBoost baseline already produced predictions in the finetune_exp_y format, but it was hardcoded to CatBoost and was not registered as an Azure job. Move the model-specific parts (construction, fit, tuning ranges, catboost device handling) into helper/baseline_models.py and select between them with a `model` config key. Logistic regression is the default; existing catboost configs pin `model: catboost` explicitly. Config-fixed hyperparameters are now also applied to the Optuna trial models, not just to the final refit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe PR adds shared logistic regression and CatBoost baseline-model handling, integrates model selection into causal training and tuning, adds baseline configurations, and exposes baseline training through an Azure job. ChangesBaseline training workflow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CausalConfig
participant main_baseline
participant baseline_models
participant Optuna
CausalConfig->>main_baseline: provide baseline model selection
main_baseline->>baseline_models: request model parameters and tuning ranges
main_baseline->>Optuna: run model-specific hyperparameter tuning
Optuna->>baseline_models: build and fit trial model
main_baseline->>baseline_models: build and fit final fold model
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@corebehrt/main_causal/helper/baseline_models.py`:
- Around line 101-110: Update build_model() to merge configured
task_type/devices with detected device settings before calling
_prepare_catboost_params(), so parameter filtering uses the effective device
mode; preserve explicit configuration precedence in the final CatBoostClassifier
arguments. Apply the same effective device-mode construction in
get_tuning_ranges() before its parameter preparation.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5ff38540-7ad5-4b86-9fad-c214842e2b37
📒 Files selected for processing (9)
corebehrt/azure/components/train_baseline.pycorebehrt/azure/main/job.pycorebehrt/configs/causal/finetune/baseline.yamlcorebehrt/configs/causal/finetune/simulated_bl.yamlcorebehrt/main_causal/helper/baseline_models.pycorebehrt/main_causal/helper/train_baseline.pyexperiments/causal_pipeline/base_configs/train_baseline.yamlexperiments/causal_pipeline_resample/base_configs/train_baseline.yamltests/test_main_causal/test_helper/test_baseline_models.py
A task_type set in the config overrides the auto-detected device in the constructor, so parameter filtering and the tuning ranges have to use the same effective device. Otherwise a config that forces GPU keeps colsample_bylevel and CatBoost errors out. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds a conventional (logistic regression) propensity score baseline to compare against the transformer PS, and wires
train_baselineup as an Azure job. Requested by reviewers: a traditional PS model as a reference, plus balance/overlap diagnostics across methods.What changed
corebehrt/main_causal/helper/baseline_models.py(new): model construction, fitting, tuning ranges and CatBoost device handling for the two baselines.helper/train_baseline.py: delegates to the factory; nested CV, folds and prediction saving unchanged.configs/causal/finetune/baseline.yaml(new): default config,model: logistic.azure/components/train_baseline.py(new) +train_baselineregistered in the job CLI.simulated_bl.yaml, bothexperiments/*/base_configs/train_baseline.yaml) pinmodel: catboostso nothing silently switches.The baseline writes
combined_predictions.csvin the same format asfinetune_exp_y, socalibrate_exp_y→estimate→get_statsrun downstream unchanged. Runningget_statswith the baseline PS gives the covariate balance / overlap / ESS comparison against the transformer.Run
Input is the same
prepared_dataasset the transformer used, so both models see identical folds.Behaviour fix
Hyperparameters pinned in the config were logged as FIXED but only reached the final refit — the Optuna trial models ignored them. They are now applied to both.
Testing
tests/test_main_causal/test_helper/test_baseline_models.py; 11 tests intests/test_main_causalpass.nested_cv_loopon synthetic patients completes for bothlogisticandcatboostand writescombined_predictions.csv.ruff format --checkand the CI lint selection are clean.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests