Skip to content

Add DerivativeCheckResult.assert_success - #78

Merged
dweindl merged 9 commits into
mainfrom
feat/derivative-check-assert-success
Aug 13, 2026
Merged

Add DerivativeCheckResult.assert_success#78
dweindl merged 9 commits into
mainfrom
feat/derivative-check-assert-success

Conversation

@dweindl

@dweindl dweindl commented Aug 7, 2026

Copy link
Copy Markdown
Member

Summary

  • Closes Add DerivativeCheckResult.assert_success #39. Ports AMICI's local assert_gradient_check_success test helper into fiddy as DerivativeCheckResult.assert_success(always_print=False).
  • No-op on a successful check (unless always_print=True); on failure raises AssertionError with a human-readable, CI-log-friendly summary: pass/fail counts, failing direction IDs, and a details table sorted by relative-difference severity (worst first) that also carries each direction's original index (so systematic patterns, e.g. "the first direction is always off", stay visible after the severity sort) and max abs/rel diff vs. tolerance.
  • Also raises (with a separate, clearly-labeled message) if the finite-difference computation itself never succeeded, before any comparison is attempted — mirroring the first check in AMICI's helper.
  • DerivativeCheckResult gained atol/rtol/derivative fields (all optional, defaulted) populated by NumpyIsCloseDerivativeCheck, and .df gained atol_success/rtol_success columns when tolerances are known.

Test plan

  • pytest tests/ — all pass (one pre-existing, unrelated timing-based flake in test_function.py::test_cache reproduces on main too and passes on rerun)
  • pre-commit run --all-files on changed files — clean
  • Manually inspected the rendered AssertionError messages for: a small failing case, a 40-direction failing case (confirms nothing is silently truncated), an always_print=True success case, and the "computation aborted" case

🤖 Generated with Claude Code

Ports AMICI's assert_gradient_check_success helper into fiddy so consumers
can drop their local copy (#39). Raises an AssertionError with
a human-readable, CI-log-friendly summary (failing direction IDs, a
worst-first diff table with each direction's original index, and max
abs/rel diff vs. tolerance) on failure, or on an unsuccessful finite-
difference computation; no-op on success unless always_print=True.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@dweindl
dweindl marked this pull request as ready for review August 7, 2026 13:39
@dweindl
dweindl requested a review from dilpath August 10, 2026 08:59

@dilpath dilpath left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

Comment thread fiddy/derivative_check.py Outdated
Comment thread fiddy/derivative_check.py Outdated
Comment thread fiddy/derivative_check.py Outdated
Comment thread fiddy/derivative_check.py
Comment thread fiddy/derivative_check.py
Comment thread fiddy/derivative_check.py Outdated
Comment thread fiddy/derivative_check.py
Comment thread fiddy/derivative_check.py
Comment thread fiddy/derivative_check.py Outdated
Comment thread tests/test_derivative.py Outdated
Comment on lines +198 to +210
def test_assert_success_shows_all_failed_directions():
point = np.full(40, 0.5)
result = _get_rosenbrock_check_result(point, rtol=0, atol=0)
assert not result.success

with pytest.raises(AssertionError) as error:
result.assert_success()

message = str(error.value)
failed_ids = result.df.index[~result.df["success"]]
assert len(failed_ids) == 40
for direction_id in failed_ids:
assert direction_id in message

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is not so interesting, I would slightly prefer a check that fails for a few specific parameters/directions and those are correctly reported

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

done

@dweindl
dweindl merged commit 145b138 into main Aug 13, 2026
5 checks passed
@dweindl
dweindl deleted the feat/derivative-check-assert-success branch August 13, 2026 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add DerivativeCheckResult.assert_success

2 participants