Skip to content

[test] Consolidate test_func helpers and add adjoint-identity harness - #1869

Open
leetcodez wants to merge 1 commit into
vgvassilev:masterfrom
leetcodez:s2-adjoint-identity
Open

[test] Consolidate test_func helpers and add adjoint-identity harness#1869
leetcodez wants to merge 1 commit into
vgvassilev:masterfrom
leetcodez:s2-adjoint-identity

Conversation

@leetcodez

@leetcodez leetcodez commented Jul 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor
  • Moved 1D helpers from stl-cmath.cpp to shared TestUtils.h to kill duplication.
  • Added multi-var central-FD helpers like fd_gradient and dot for independent checking.
  • Added DotProductIdentity.cpp harness to test forward vs reverse vs FD agreementon multi-variable targets under both default and -enable-ua paths.
  • This establishesa baseline gradient testing harness to catch regressions in upcoming activity-analysis PRs.

@github-actions

github-actions Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@codecov

codecov Bot commented Jul 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@leetcodez
leetcodez force-pushed the s2-adjoint-identity branch from fb1fd40 to d69cafb Compare July 4, 2026 10:14
@leetcodez leetcodez changed the title Add adjoint dot-product identity checks to TestUtils [tests] Add tangent/adjoint dot-product identity checks to TestUtils Jul 4, 2026
@leetcodez leetcodez changed the title [tests] Add tangent/adjoint dot-product identity checks to TestUtils [test] Add tangent/adjoint dot-product identity checks to TestUtils Jul 4, 2026
@github-actions

github-actions Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@vgvassilev

vgvassilev commented Jul 4, 2026

Copy link
Copy Markdown
Owner

What is the reason for this change?

@leetcodez

leetcodez commented Jul 4, 2026

Copy link
Copy Markdown
Contributor Author

What is the reason for this change?

The goal was to add a test harness that catches silently wrong gradients by cross-checking reverse mode against forward mode (they're computed independently).
Currently clad's gradient tests only check handwritten expected values on a few cases, this adds an automatic check that generalizes to any function.

@leetcodez
leetcodez force-pushed the s2-adjoint-identity branch from d69cafb to 68e8fd9 Compare July 4, 2026 12:48
@github-actions

github-actions Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@leetcodez
leetcodez force-pushed the s2-adjoint-identity branch from a8b4f64 to 2631ceb Compare July 4, 2026 14:32
@vgvassilev

vgvassilev commented Jul 4, 2026

Copy link
Copy Markdown
Owner

https://github.com/vgvassilev/clad/blob/master/test/Features/stl-cmath.cpp has similar infrastructure. This seems to be duplicating parts of it.

@github-actions

github-actions Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@leetcodez
leetcodez force-pushed the s2-adjoint-identity branch from 2631ceb to 144f4ec Compare July 4, 2026 16:56
@vgvassilev

vgvassilev commented Jul 4, 2026

Copy link
Copy Markdown
Owner

I don’t see how the newly added test is improving coverage. These test utils should be used in the context of actual changes.

@leetcodez
leetcodez marked this pull request as draft July 4, 2026 17:26
@leetcodez
leetcodez force-pushed the s2-adjoint-identity branch from 144f4ec to 15ad98f Compare July 5, 2026 07:00
@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@leetcodez
leetcodez marked this pull request as ready for review July 5, 2026 07:13
@leetcodez
leetcodez marked this pull request as draft July 5, 2026 07:13
@leetcodez
leetcodez force-pushed the s2-adjoint-identity branch from 15ad98f to 2d8c834 Compare July 5, 2026 08:48
@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@leetcodez
leetcodez force-pushed the s2-adjoint-identity branch from 250f1d7 to a6b91e0 Compare July 7, 2026 09:38
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

1 similar comment
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@leetcodez
leetcodez force-pushed the s2-adjoint-identity branch from a6b91e0 to 4ceb994 Compare July 7, 2026 09:47
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@leetcodez
[test] Consolidate test_func helpers and add adjoint-identity harness
94ee4f3 
Moved 1D helpers from stl-cmath.cpp to shared TestUtils.h to kill duplication.
Added multi-var central-FD helpers like fd_gradient and dot for independent checking.
Added DotProductIdentity.cpp harness to test forward vs reverse vs FD agreement
on multi-variable targets under both default and -enable-ua paths. This establishes
a baseline gradient testing harness to catch regressions in upcoming activity-analysis PRs.
@leetcodez
leetcodez force-pushed the s2-adjoint-identity branch from 4ceb994 to 94ee4f3 Compare July 7, 2026 10:05
@leetcodez leetcodez changed the title [test] Add tangent/adjoint dot-product identity checks to TestUtils [test] Consolidate test_func helpers and add adjoint-identity harness Jul 7, 2026
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@leetcodez
leetcodez marked this pull request as ready for review July 11, 2026 10:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@leetcodez @vgvassilev