Adjust correctly the level of metrics calculated during cross-validation - #840
Open
andyreus17 wants to merge 13 commits into
Open
Adjust correctly the level of metrics calculated during cross-validation#840andyreus17 wants to merge 13 commits into
andyreus17 wants to merge 13 commits into
Conversation
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.
Summary
The base branch already carved a partition out of cross-validation, but it only fed explainability: it produced no metric at all. At the same time the folds still stored their results under the
testname, even though a fold score is an estimate obtained by resampling.That combination had a concrete consequence. The model comparison table showed a column labelled "Test" that actually held averaged validation scores. Since that column is what the user reads to pick a model, the value stops being an unbiased estimate of generalization the moment it is used to select. So, now cross validation has
validationmetrics (obtained from the multiple folds evaluation) andtrainmetrics, obtained from the independent set, thereby aligning itself with the standard of literature (validation metrics for model selection and test metrics for model assessment).In short, the following changes were made:
validation, which is what they are.test, the only estimate that no fold and no hyperparameter search ever influenced.testname free, theEVALUATION_PARTITIONindirection that existed to avoid the name collision is removed.holdouttotest_size, matching what it does and no longer clashing withHoldoutSplitter/HoldoutEvaluationStrategy.Behaviour with
test_size = 0is unchanged from the base branch (apply cross validation over all data).EXTRA (fixed existing bug): Disabling hyperparameter optimization on a run left its old optimizer, optimizer parameters, goal metric and nested-CV config still attached and showing in the "Model" sidebar, since nothing cleared that state when the
optimizeflags were unchecked. The edit dialog now clears all four when no parameter is marked for optimization.Type of Change
Check all that apply like this [x]:
Changes (by file)
Backend
DashAI/back/evaluation/cv.py: Folds writeSplitEnum.VALIDATIONinstead ofTESTatFOLDlevel; the HPO objective inevaluate()and itsTRIALmetrics move to validation, as does the outer loop of_nested_cv()atOUTER_FOLDlevel. A new STEP 5 scores the reserved rows once with the final model and stores the result asTESTatLASTlevel, without a standard deviation.DashAI/back/splitters/fold_splitter.py:validationinstead oftestindexes and asks for a"validation"partition when building theDatasetDicts. Thefull_datasetentry keeps{train, test}with the reserved rows.explainable_partitions()returns{"train", "test"}instead of{"train", "holdout"}, andEVALUATION_PARTITIONis removed.holdout->test_size,HOLDOUT_STRATEGY->TEST_SPLIT_STRATEGY,_carve_holdout()->_carve_test_split()select()method to filter dataset rows is changed forsplit_dataset_cv()due to a slow performance of the first method. That way,split()method of fold based splitters went from ~35s to ~0.06s (using the student dataset).DashAI/back/splitters/base_splitter.py: TheEVALUATION_PARTITIONattribute is removed. The gate inexplainable_splits()checks thetestpartition directly.DashAI/back/splitters/k_fold.py,stratified_k_fold.py,group_k_fold.py,stratified_group_k_fold.py,repeated_k_fold.py,repeated_stratified_k_fold.py,leave_one_out.py:holdout->test_size.DashAI/back/splitters/splits_payload.py:explainable_indexes()resolves the evaluation slot withpartitions.get("test").DashAI/back/dataloaders/classes/dashai_dataset.py:split_dataset_cv()acceptssecond_split_name(default"test"), so folds return{train, validation}while the trailing element keeps{train, test}.DashAI/back/api/api_v1/endpoints/runs.py:get_metrics_for_run()builds its response by iteratingSplitEnuminstead of listing keys by hand.attach_metrics_to_run()replaces the two duplicated assignment blocks inget_runsandget_run_by_id.fold-metricsgoes fromLiteral["train","test"]defaulting to"test"toLiteral["train","validation"]defaulting to"validation", since no test metric exists at fold level anymore.reset_run()also clears the_stdfields.DashAI/back/api/api_v1/endpoints/explainers.py: Docstring of theexplainable-splitsendpoint: a cross-validation run no longer exposes its reserved rows under the nameholdout.DashAI/back/job/explainer_job.py:valid_splitsbecomes["train", "val", "all", "test"].DashAI\back\api\api_v1\endpoints\runs.py:update_runkeys thenestedupdate offmodel_fields_setinstead of the value, so an explicitnullclears the nested CV config instead of being indistinguishable from an omitted field. The modification guard moved fromany([...])tomodel_fields_setas well, since truthiness read a cleared optimizer (""/{}) as "nothing changed".Frontend
DashAI/front/src/utils/splitsPayload.js:hasPartition()asserted that cross-validation never builds a validation partition. Now In CV,trainandvalidationnow always receive rows, andtestdoes only whentest_size > 0.DashAI/front/src/components/models/AddModelDialog.jsxandRunEditForm.jsx:maxInnerFoldsdiscounts the reserved proportion since nested CV inner folds are built over the pool, not over the whole dataset.DashAI/front/src/components/models/runResults/ResultsTabsHeader.jsx: Readstest_sizeinstead ofholdoutwhen deciding whether the run has explainable data.DashAI/front/src/components/models/ModelComparisonTable.jsx: The+/-no longer depends on the evaluation strategy but on the value being present. The validation column shows it and the test column does not, with no conditional branch.DashAI/front/src/pages/results/components/ResultsTabMetricsToggle.jsxandResultsTabMetrics.jsx: The toggle becomes data-driven throughhasTrainData/hasValidationData/hasTestDatainstead of hiding validation whenever the strategy is CV.ResultsTabMetricsderives them from the run's metrics.DashAI/front/src/components/models/LiveMetricsChart.jsx: The validation tab was conditioned on the session not being CV. It is now always shown.DashAI/front/src/components/models/FoldMetricsChart.jsx: The buttonTESTbecomesVALIDATION.DashAI/front/src/components/models/OuterFoldMetricsTable.jsx: Readsvalidation_metrics/validation_metrics_std, which is where nested CV now stores its outer fold results.DashAI/front/src/components/models/StatisticalTestsModal.jsx: Default splitvalidation, readsrun.validation_metrics, and the selector is reduced to train/validation: a paired test needs one sample per fold, and the reserved rows yield a single value.DashAI/front/src/utils/i18n/locales/{en,es,pt,de,zh}/common.json: Theholdoutkey is removed because it was not being used.DashAI\front\src\hooks\useRunEditForm.js:doSave()now derives the payload fromhasOptimizableParams, sending empty values for the optimizer, its parameters and the goal metric, andnullfor the nested config, when no parameter is marked for optimization.isDirtycompared the inner-CV config against the full stored splitter config, two different shapes that never matched, leaving Save permanently enabled. It now compares against the normalized shape, and only while nested CV is on.Tests
tests/back/splitters/test_holdout_carve.py->test_carve_test_split.pyRenamed along with its functions and variables. Assertions on the folds move to
validation_indexesand to thevalidationkey of theDatasetDict, and the configuration usestest_size.tests/back/splitters/test_explainable_partitions.pyProbeSplitterand the two tests that used it are removed: they existed to prove a splitter could freely name its unseen partition. Now CV asserts the reserved rows are offered astest; the refusal test is unified because both families now fail with the same message.tests/back/api/test_explainer_jobs.pyThe endpoint assertion goes from
{"name": "holdout"}to{"name": "test"}, and the file's helpers drop the previous vocabulary.