Repository navigation
Conversation
5 tasks done
SvenKlaassen
self-requested a review
November 3, 2026 06:58
This branch has not been deployed
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.
The LPLR Optuna tests compare post-tuning two-fold DoubleML predictions with their untuned counterparts, although tuning optimizes a separate five-fold CV objective. They also compare classifier RMSE even though the default classifier tuning objective is log loss. Improvement in these different evaluations is not guaranteed.
On current main, four cases with an explicitly supplied
ml_afail. For example, the two-fold RMSE worsens from 0.6979 to 0.7056, while the actual five-fold tuning RMSE improves from 0.7212 to 0.6600. Independently recomputing the selected model's score with scikit-learn agrees with the reported Optuna score.This test-only correction:
ml_aestimator on the public inner splits and compares its averaged predictions with DoubleML's stored predictions, verifying that fitting actually uses the chosen parameters.No estimator or tuning production code changes.
Reproduce with
pytest doubleml/plm/tests/test_lplr_tune_ml_models.py -q: 4 failed, 4 passed before; 8 passed after. Formatting and lint checks pass. Independent automated review found no blocking issues. Complete-suite validation of the combined branch in #445 passed 11,613 cases with 66 intentional skips and four worker-process crashes; all four affected cases passed on serial retry. All eight LPLR tuning cases passed in that full run.This arose while validating #445, which also includes this test-fix commit so it can be validated with the correction in place. The fix can be reviewed and merged independently first.