-
Notifications
You must be signed in to change notification settings - Fork 121
Improved error reporting for failed tolerance checks #988
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
e31d6a0
93053e1
8f9793f
a84e311
c27c941
c817dfe
9123f8b
35fbc7e
dd38696
6329b04
a7f38f2
34c618a
dda29ac
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -18,6 +18,34 @@ def is_close(error: Error, tolerance: Tolerance) -> bool: | |||||||||||||||||||||
| return False | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| def _format_error_diagnostics(max_abs_info, max_rel_info) -> str: | ||||||||||||||||||||||
| """Format diagnostic information for maximum errors among failing variables.""" | ||||||||||||||||||||||
| diagnostic_msg = "" | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| if max_abs_info: | ||||||||||||||||||||||
| filepath, val_idx, g_val, c_val, abs_err, rel_err = max_abs_info | ||||||||||||||||||||||
| rel_error_str = f"{rel_err:.2E}" if not math.isnan(rel_err) else "NaN" | ||||||||||||||||||||||
| diagnostic_msg += f"\n\nDiagnostics - Maximum absolute error among FAILING variables:\n" \ | ||||||||||||||||||||||
| f" - File: {filepath}\n" \ | ||||||||||||||||||||||
| f" - Variable n°{val_idx+1}\n" \ | ||||||||||||||||||||||
| f" - Candidate: {c_val}\n" \ | ||||||||||||||||||||||
| f" - Golden: {g_val}\n" \ | ||||||||||||||||||||||
| f" - Absolute Error: {abs_err:.2E}\n" \ | ||||||||||||||||||||||
| f" - Relative Error: {rel_error_str}" | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| if max_rel_info: | ||||||||||||||||||||||
| filepath, val_idx, g_val, c_val, rel_err, abs_err = max_rel_info | ||||||||||||||||||||||
| diagnostic_msg += f"\n\nDiagnostics - Maximum relative error among FAILING variables:\n" \ | ||||||||||||||||||||||
| f" - File: {filepath}\n" \ | ||||||||||||||||||||||
| f" - Variable n°{val_idx+1}\n" \ | ||||||||||||||||||||||
| f" - Candidate: {c_val}\n" \ | ||||||||||||||||||||||
| f" - Golden: {g_val}\n" \ | ||||||||||||||||||||||
| f" - Relative Error: {rel_err:.2E}\n" \ | ||||||||||||||||||||||
| f" - Absolute Error: {abs_err:.2E}" | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| return diagnostic_msg | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # pylint: disable=too-many-return-statements | ||||||||||||||||||||||
| def compare(candidate: Pack, golden: Pack, tol: Tolerance) -> typing.Tuple[Error, str]: | ||||||||||||||||||||||
| # Keep track of the average error | ||||||||||||||||||||||
|
|
@@ -45,23 +73,69 @@ def compare(candidate: Pack, golden: Pack, tol: Tolerance) -> typing.Tuple[Error | |||||||||||||||||||||
| error = compute_error(cVal, gVal) | ||||||||||||||||||||||
| avg_err.push(error) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| def raise_err(msg: str): | ||||||||||||||||||||||
| def raise_err_with_failing_diagnostics(msg: str): | ||||||||||||||||||||||
| # Find maximum errors among FAILING variables only | ||||||||||||||||||||||
| max_abs_info, max_rel_info = find_maximum_errors_among_failing(candidate, golden, tol) | ||||||||||||||||||||||
| diagnostic_msg = _format_error_diagnostics(max_abs_info, max_rel_info) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| return None, f"""\ | ||||||||||||||||||||||
| Variable n°{valIndex+1} (1-indexed) in {gFilepath} {msg}: | ||||||||||||||||||||||
| - Candidate: {cVal} | ||||||||||||||||||||||
| - Golden: {gVal} | ||||||||||||||||||||||
| - Error: {error} | ||||||||||||||||||||||
| - Tolerance: {tol} | ||||||||||||||||||||||
| - Tolerance: {tol}{diagnostic_msg} | ||||||||||||||||||||||
| """ | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| if math.isnan(gVal): | ||||||||||||||||||||||
| return raise_err("is NaN in the golden file") | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| return raise_err_with_failing_diagnostics("is NaN in the golden file") | ||||||||||||||||||||||
| if math.isnan(cVal): | ||||||||||||||||||||||
| return raise_err("is NaN in the pack file") | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| return raise_err_with_failing_diagnostics("is NaN in the pack file") | ||||||||||||||||||||||
| if not is_close(error, tol): | ||||||||||||||||||||||
| return raise_err("is not within tolerance") | ||||||||||||||||||||||
| return raise_err_with_failing_diagnostics("is not within tolerance") | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # Return the average relative error | ||||||||||||||||||||||
| return avg_err.get(), None | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| def find_maximum_errors_among_failing(candidate: Pack, golden: Pack, tol: Tolerance) -> typing.Tuple[typing.Optional[typing.Tuple[str, int, float, float, float, float]], typing.Optional[typing.Tuple[str, int, float, float, float, float]]]: | ||||||||||||||||||||||
| """ | ||||||||||||||||||||||
| Scan all files to find the maximum absolute and relative errors among FAILING variables only. | ||||||||||||||||||||||
| A variable fails if is_close(error, tol) returns False. | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Returns tuple of: | ||||||||||||||||||||||
| - max_abs_info: (filepath, var_index, golden_val, candidate_val, absolute_error, relative_error) | ||||||||||||||||||||||
| - max_rel_info: (filepath, var_index, golden_val, candidate_val, relative_error, absolute_error) | ||||||||||||||||||||||
| """ | ||||||||||||||||||||||
| max_abs_error = -1.0 | ||||||||||||||||||||||
| max_abs_info = None | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| max_rel_error = -1.0 | ||||||||||||||||||||||
|
||||||||||||||||||||||
| max_rel_error = -1.0 | |
| max_abs_error = float('-inf') | |
| max_abs_info = None | |
| max_rel_error = float('-inf') |
Copilot
AI
Aug 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same issue as with max_abs_error - using -1.0 as initial value is unclear. Consider using float('-inf') or None with explicit checks for better clarity.
| max_rel_error = -1.0 | |
| max_abs_error = float('-inf') | |
| max_abs_info = None | |
| max_rel_error = float('-inf') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The return type annotation uses the same deeply nested tuple structure as format_diagnostic_message. This complex type signature makes the code harder to understand and maintain. Consider using named tuples or dataclasses.