fix: report an undefined cross-sectional IC as null, not NaN (release 0.1.2) - #44
Merged
Conversation
`cross_sectional_ic_series` builds its per-date IC with `pl.corr` over
ranks. Spearman and Pearson are both undefined when one side of a date
has zero variance - every prediction tied, or every return tied - and
`pl.corr` returns NaN there. Everything else in the same function already
uses null for a date it cannot compute: the `min_obs` gate writes
`.otherwise(None)`, and `cross_sectional_ic` reports such a date as
missing in `n_periods`. Only the raw `pl.corr` result escaped as NaN.
Polars treats NaN and null as different values, so `drop_nulls("ic")`
keeps the NaN and `.mean()` over the survivors is NaN. A weak model ties
on some dates, so a caller's headline metric silently becomes nan and
nothing raises. Measured in `12_gradient_boosting/07_hpo_comparison`
fold 0: 231 validation dates, 0 nulls, 52 NaN.
Gate the correlation on `is_finite()` alongside `min_obs`, so an
undefined date carries null. The three-date reproducer from the issue:
date n_obs ic (before) ic (after)
2024-01-01 12 -0.097902 -0.097902
2024-01-02 12 NaN null
2024-01-03 12 -0.545455 -0.545455
drop_nulls("ic").mean(): nan -> -0.321678
`n_periods` is 2 both ways; the aggregate wrapper already counted the
tied date as missing.
Tests cover both correlation methods against both tied sides, asserting
`null_count() == 1` and `is_nan().sum() == 0`. All four fail when the
`is_finite()` gate is removed.
Refs: stefan-jansen/machine-learning-for-trading#493
Carries the #493 fix: an undefined cross-sectional IC date now reports
null instead of NaN, so a `drop_nulls("ic").mean()` in a caller cannot
come back nan.
`[tool.hatch.version]` reads `__version__` from
`src/ml4t/diagnostic/__init__.py`, not from the git tag, so the bump has
to land on a commit before `v0.1.2` is tagged - tagging alone rebuilds
0.1.1 and PyPI rejects the upload as a duplicate. That is what happened
to the first v0.1.1 tag (see 3467a11).
The CHANGELOG had no 0.1.1 entry; added one from that release commit so
the file does not jump 0.1.0b21 -> 0.1.2.
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.
Closes #43.
The fix
cross_sectional_ic_series()builds its per-date IC withpl.corrover ranks.The correlation is undefined when one side of a date has zero variance - every
prediction tied, or every return tied - and
pl.corrreturns NaN. Polars treatsNaN and null as different values, so the NaN survives
drop_nulls("ic")and thecaller's mean IC comes back
nanwith nothing raising.The correlation is now gated on
is_finite()alongsidemin_obs, so anundefined date carries null - the same as a date below
min_obs, and the sameas what
cross_sectional_ic()already reported inn_periods.Verified end to end against the published 0.1.1 wheel with the issue's
reproducer:
icon the tied dateNaNnulldrop_nulls("ic").mean()nan0.244755n_periodsFour new tests cover both correlation methods against both tied sides, asserting
null_count() == 1andis_nan().sum() == 0. All four fail when theis_finite()gate is removed.Release 0.1.2
[tool.hatch.version]reads__version__fromsrc/ml4t/diagnostic/__init__.py, not from the git tag, so the bump has to landon a commit before
v0.1.2is tagged - tagging alone would rebuild 0.1.1 andPyPI would reject the upload as a duplicate. That is what happened to the first
v0.1.1tag.The CHANGELOG had no 0.1.1 entry; one is added from that release commit so the
file does not jump 0.1.0b21 -> 0.1.2.
Local verification
ruff check,ruff format --check, andty checkclean; 5331 passed, 75skipped.
uv buildplusscripts/verify_release_artifacts.pyandscripts/import_smoke.pypass against the built 0.1.2 wheel.