Skip to content

Add a test mode for forward-on-reverse#796

Closed
kshyatt wants to merge 8 commits intochalk-lab:mainfrom
kshyatt:ksh/uturn
Closed

Add a test mode for forward-on-reverse#796
kshyatt wants to merge 8 commits intochalk-lab:mainfrom
kshyatt:ksh/uturn

Conversation

@kshyatt
Copy link
Collaborator

@kshyatt kshyatt commented Oct 2, 2025

This mode is helpful for testing rules where the choice of gauge for outputs (e.g. for eigenvectors) is meaningful. Finite differences will pick an arbitrary gauge which leads to "false" correctness failures in the rule tests. This PR introduces a new mode that runs the reverse mode first (the output tangent allows a particular choice of gauge), checks it with FD, then uses that to check the forward rule.

@codecov
Copy link

codecov bot commented Oct 2, 2025

Codecov Report

❌ Patch coverage is 22.72727% with 17 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/test_utils.jl 22.72% 17 Missing ⚠️

📢 Thoughts on this report? Let us know!

@kshyatt
Copy link
Collaborator Author

kshyatt commented Oct 3, 2025

OK, so I have the tests for the x's passing, which I think is correct and not testing that a === a 😅 . But my tests for the tangents of y is not working even for a rule I'm pretty confident in -- not sure what I've done wrong here but probably something!

@yebai yebai requested a review from sunxd3 October 10, 2025 07:38
yebai and others added 3 commits October 13, 2025 16:32
Signed-off-by: Hong Ge <3279477+yebai@users.noreply.github.com>
@yebai
Copy link
Member

yebai commented Oct 13, 2025

@sunxd3, can you help review this?

ẏ_ad = tangent(y_ẏ_rule)

# now use ẏ_ad as output_tangent for rrule again
test_rrule_correctness(
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm a little confused by why we're running reverse-mode again here. Would you mind running be through the logic of what's going on here again @kshyatt ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I have similar question here. It feels like to me that the forward rule is not tested?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The idea is to generate the derivatives from the forward pass then use them as the outputs to the reverse pass (to ensure you stay in the correct gauge) -- but it's very possible I did this wrong, I find the whole testing framework very hard to make sense of to be honest

Copy link
Member

Choose a reason for hiding this comment

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

@AstitvaAggarwal, can you help?

kshyatt and others added 3 commits October 17, 2025 11:37
Co-authored-by: Xianda Sun <5433119+sunxd3@users.noreply.github.com>
Signed-off-by: Katharine Hyatt <kshyatt@users.noreply.github.com>
Co-authored-by: Xianda Sun <5433119+sunxd3@users.noreply.github.com>
Signed-off-by: Katharine Hyatt <kshyatt@users.noreply.github.com>
Co-authored-by: Xianda Sun <5433119+sunxd3@users.noreply.github.com>
Signed-off-by: Katharine Hyatt <kshyatt@users.noreply.github.com>
@yebai
Copy link
Member

yebai commented Nov 4, 2025

@kshyatt, would you be open to combining this PR with #833?

@kshyatt
Copy link
Collaborator Author

kshyatt commented Nov 5, 2025 via email

@yebai
Copy link
Member

yebai commented Nov 10, 2025

Closed in favour of #808

@yebai yebai closed this Nov 10, 2025
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.

4 participants