Skip to content

Make the data-leakage trigger reachable - #8

Open
GuoCheng24 wants to merge 1 commit into
InternScience:mainfrom
GuoCheng24:fix-leakage-trigger-threshold
Open

Make the data-leakage trigger reachable#8
GuoCheng24 wants to merge 1 commit into
InternScience:mainfrom
GuoCheng24:fix-leakage-trigger-threshold

Conversation

@GuoCheng24

Copy link
Copy Markdown

should_check_data_leakage compares the metric with == 1.0 / == 0.0, so the check it guards — check_data_leakage, which is on by default in config/config.yaml — effectively never runs. Floating-point metrics do not land on exact 1.0 or 0.0.

Reproduced against agents/triggers.py on main:

AUC leaked        value=0.99997   maximize=True   -> fires: False
accuracy leaked   value=0.9993    maximize=True   -> fires: False
RMSE leaked       value=1.2e-08   maximize=False  -> fires: False
logloss leaked    value=0.00031   maximize=False  -> fires: False

So the 117-line agents/data_leakage_agent.py behind it is dead code in practice. That matters here because leakage is the main way an ML-engineering agent fakes a medal on MLE-bench.

The change. Compare against configurable bounds instead of exact equality, chosen conservatively so the old exact-1.0 / exact-0.0 cases stay inside the new bounds and ordinary scores still do not fire:

case before after
AUC 0.99997 no yes
accuracy 0.9993 no yes
RMSE 1.2e-08 no yes
exactly 1.0 / 0.0 yes yes (unchanged)
normal AUC 0.87 no no
normal RMSE 0.31 no no

Defaults are leakage_max_threshold: 0.999 and leakage_min_threshold: 1e-6 — deliberately tight, since a default-on guard must not cry wolf. A leaked logloss of 3.1e-4 does not fire at the default and does at 1e-3; the threshold is there to be tuned per metric family.

Note on the three-file diff. config/__init__.py builds the authoritative schema with OmegaConf.structured(Config) before merging the YAML, so a key present only in config.yaml raises ConfigKeyError. Both files must declare the new fields; I verified that adding them to the YAML alone fails, and that the merge succeeds with both. The fields sit after use_aggregation because dataclass ordering requires defaulted fields to follow non-defaulted ones.

should_check_data_leakage compares the metric with == 1.0 / == 0.0, so the
check it guards (check_data_leakage, on by default) effectively never runs:
a leaked AUC lands on 0.99997 and a leaked RMSE on 1.2e-08. Compare against
configurable bounds instead, chosen conservatively so the exact-1.0 and
exact-0.0 cases stay inside the new bounds and ordinary scores do not fire.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant