fix: count/nunique aggregates yield 0, not NaN, for parents without children#14
Open
CristobalSantana wants to merge 1 commit into
Open
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
Author
|
I have read the CLA Document and I hereby sign the CLA |
abafc91 to
474a604
Compare
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.
What
In a one_to_many relation, aggregates are computed by grouping the child table and left-merging onto the parent. Parents with no child rows therefore receive NaN in every aggregate column, including count and nunique. Downstream, preprocessing bins those NaNs as missing.
The count of an empty set is 0, not a missing value. Leaving it as NaN conflates "no children exist" (a real, often highly informative value) with "data is missing", and the two become indistinguishable in the trained network. This felt like exactly the kind of hidden behavior the design principles ask contributors to avoid.
How I found it
I stress-tested the library with a synthetic public-tenders dataset (3,000 tenders, 8,000 bids, target = tender declared void). 287 tenders had zero bids, which was by construction the strongest predictor of the target (80% positive rate vs 22% global). After materialization, all 287 had bid_count = NaN and were binned as missing. With the fix, on that dataset, ROC-AUC went from 0.753 to 0.808 and PR-AUC from 0.533 to 0.608. The metric gain is dataset-specific, but the semantic issue is general: in real data that also has genuinely missing child records, the two cases cannot be separated at all without this distinction.
Change
After the aggregate merge, fill NaN with 0 only for columns produced by count and nunique ops. Other ops (mean, latest, etc.) still yield NaN, since they are truly undefined over an empty set. This also covers windowed counts (window_days), where a parent with children outside the window now gets 0 instead of NaN.
Note this is a behavior change for anyone relying on NaN counts; given 0.1.0 alpha status I assumed the semantic fix is preferred, but happy to gate it behind a config flag if you'd rather keep the current default.
Testing
New regression test: parent with zero children gets count == 0, nunique == 0, and mean stays NaN. uv run ruff check . clean, full test suite passes (63 tests), smoke test auto-bayesian train examples/lead_scoring.toml runs.