Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 81 additions & 7 deletions toolchain/mfc/packer/tol.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Copy link

Copilot AI Aug 18, 2025

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.

Suggested change
- max_rel_info: (filepath, var_index, golden_val, candidate_val, relative_error, absolute_error)
def format_diagnostic_message(max_errors: Tuple[Optional[ErrorInfo], Optional[ErrorInfo]]) -> str:
"""Format the diagnostic message showing maximum errors."""
max_abs_info, max_rel_info = max_errors
diagnostic_msg = ""
if max_abs_info:
rel_error_str = f"{max_abs_info.relative_error:.2E}" if not math.isnan(max_abs_info.relative_error) else "NaN"
diagnostic_msg += f"\n\nDiagnostics - Maximum absolute error across ALL files:\n" \
f" - File: {max_abs_info.filepath}\n" \
f" - Variable n°{max_abs_info.var_index+1}\n" \
f" - Candidate: {max_abs_info.candidate_val}\n" \
f" - Golden: {max_abs_info.golden_val}\n" \
f" - Absolute Error: {max_abs_info.absolute_error:.2E}\n" \
f" - Relative Error: {rel_error_str}"
if max_rel_info:
diagnostic_msg += f"\n\nDiagnostics - Maximum relative error across ALL files:\n" \
f" - File: {max_rel_info.filepath}\n" \
f" - Variable n°{max_rel_info.var_index+1}\n" \
f" - Candidate: {max_rel_info.candidate_val}\n" \
f" - Golden: {max_rel_info.golden_val}\n" \
f" - Relative Error: {max_rel_info.relative_error:.2E}\n" \
f" - Absolute Error: {max_rel_info.absolute_error:.2E}"
return diagnostic_msg
def find_maximum_errors(candidate: Pack, golden: Pack) -> Tuple[Optional[ErrorInfo], Optional[ErrorInfo]]:
"""
Scan all files to find the maximum absolute and relative errors.
Returns tuple of:
- max_abs_info: ErrorInfo or None
- max_rel_info: ErrorInfo or None

Copilot uses AI. Check for mistakes.
"""
max_abs_error = -1.0
max_abs_info = None

max_rel_error = -1.0
Copy link

Copilot AI Aug 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using -1.0 as an initial value for maximum error tracking is unclear and could be problematic if all errors are actually negative (though unlikely in this context). Consider using float('-inf') or None with explicit checks for better clarity.

Suggested change
max_rel_error = -1.0
max_abs_error = float('-inf')
max_abs_info = None
max_rel_error = float('-inf')

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Aug 18, 2025

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.

Suggested change
max_rel_error = -1.0
max_abs_error = float('-inf')
max_abs_info = None
max_rel_error = float('-inf')

Copilot uses AI. Check for mistakes.
max_rel_info = None

for gFilepath, gEntry in golden.entries.items():
cEntry = candidate.find(gFilepath)
if cEntry is None:
continue

for valIndex, (gVal, cVal) in enumerate(zip(gEntry.doubles, cEntry.doubles)):
# Skip NaN values in golden or candidate
if math.isnan(gVal) or math.isnan(cVal):
continue

error = compute_error(cVal, gVal)

# Only consider variables that FAIL tolerance
if is_close(error, tol):
continue # Skip variables that pass tolerance

# Track maximum absolute error among failing variables
if error.absolute > max_abs_error:
max_abs_error = error.absolute
max_abs_info = (gFilepath, valIndex, gVal, cVal, error.absolute, error.relative)

# Track maximum relative error among failing variables (only if it's not NaN)
if not math.isnan(error.relative) and error.relative > max_rel_error:
max_rel_error = error.relative
max_rel_info = (gFilepath, valIndex, gVal, cVal, error.relative, error.absolute)

return max_abs_info, max_rel_info
Loading