Feat/structured comparison result - #22
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1cd92ec317
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| value = run.summary_metrics[metric] | ||
| if value is None: | ||
| missing_value_run_ids.append(run.run_id) | ||
| continue |
There was a problem hiding this comment.
When a source reports NaN or infinity, which the canonical float fields currently permit, this check treats the value as usable because it rejects only None. A NaN pair then produces NaN aggregates while being counted as non-positive, yielding a misleading positive_pair_rate of zero; infinities can similarly create invalid differences. Reject non-finite values with a ComparisonError before indexing them.
Useful? React with 👍 / 👎.
| mean_difference=sum(raw_differences) / len(raw_differences), | ||
| median_difference=median(raw_differences), | ||
| mean_improvement=sum(improvements) / len(improvements), | ||
| median_improvement=median(improvements), |
There was a problem hiding this comment.
For paired differences with mixed signs and substantially different magnitudes, ordinary sum can lose smaller terms and report the wrong mean; for example, differences [1e16, 1, -1e16] produce 0.0 here instead of approximately 0.333. Because mean effect is a primary comparison result, use a numerically stable summation such as math.fsum for both raw differences and improvements.
Useful? React with 👍 / 👎.
1 participant
Problem
What situation existed before this change? Why did it need to change?
Change
What did you actually do? Summarize the change, not the diff line-by-line.
Design Decisions
Any non-obvious choices made while implementing this — trade-offs, rejected
alternatives, or anything a future reader would ask "why did you do it this
way?" about.
Validation
pytestpassesruff check .passesmypy srcpassesRisks
What could this break? What wasn't tested? What assumptions might not hold
for other inputs or backends?
Related