-
Notifications
You must be signed in to change notification settings - Fork 62
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
Tests II: more Julia function tests #969
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #969 +/- ##
==========================================
- Coverage 75.39% 74.64% -0.75%
==========================================
Files 35 35
Lines 10653 10650 -3
==========================================
- Hits 8032 7950 -82
- Misses 2621 2700 +79 ☔ View full report in Codecov by Sentry. |
@jgreener64 can you isolate the 1.8 segfault into an issue? |
test/runtests.jl
Outdated
# On Julia 1.6 the gradients are wrong (0.7 not 1.2) and on 1.7 it errors | ||
@static if VERSION ≥ v"1.8-" | ||
# On Julia 1.6 the gradients are wrong (0.7 not 1.2) | ||
@static if VERSION ≥ v"1.7-" |
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.
Open a bug for this?
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.
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.
remove the version now?
Can you isolate that 1.7 assertion error or segfault? |
I can't reproduce the Julia 1.7 issue locally, pushing some print statements to isolate it in CI. |
test/runtests.jl
Outdated
return 2x | ||
end | ||
end | ||
@test autodiff(Reverse, f25, Active, Active(2.0))[1][1] == 2 |
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.
add the test_broken for 1.6?
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.
Done, unfortunately the @test 1==1 broken=cond
syntax is not available on Julia 1.6 so we have to use an if
and @test_broken
.
By the way I can remove the print statements just before this is merged, they are for debugging.
I think the only unresolved issue is #970, I could remove that test to merge this PR. |
test/runtests.jl
Outdated
f26(x) = circshift([1.0, 2x, 3.0], 1)[end] | ||
@test autodiff(Reverse, f26, Active, Active(2.0))[1][1] == 2 | ||
@test autodiff(Forward, f26, Duplicated(2.0, 1.0))[1] == 2 | ||
println("Done 26") |
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.
remove these prints?
I added tests for more Julia functions, including some which have had Enzyme issues fixed recently. I also added a nested reverse mode test and turned on previous tests that now pass on older Julia versions.
All pass on my machine, let's see what CI says.