-
Notifications
You must be signed in to change notification settings - Fork 64
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
ci: add DynamicExpressions integration tests #1675
Conversation
09eb2e6
to
295ac6a
Compare
SGTM! |
768d692
to
628038e
Compare
Cool, seems to work! (I think the #1674 is a different issue) |
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 #1675 +/- ##
===========================================
+ Coverage 70.60% 96.35% +25.74%
===========================================
Files 31 9 -22
Lines 12926 411 -12515
===========================================
- Hits 9127 396 -8731
+ Misses 3799 15 -3784 ☔ View full report in Codecov by Sentry. |
DynamicExpressions.jl seems to be good at flagging issues in Enzyme, such as the below, so this PR therefore adds a simple integration test for it so that these issues will get flagged immediately.
Error: Enzyme aligned size and Julia size disagree
#999bitcast!
#1674The CI also pins DynamicExpressions to a specific version where it has been known to work, so any failures would indicate some kind of internal regression. Also, this is a separate CI run so won't be recorded in the main test suite.
This is also important for the stability of the PySR/SymbolicRegression.jl ecosystem which have a large number of non-technical users; I would want to prevent breakages for them when I eventually make Enzyme part of the default search code.
I restructured the CI so that packages can add their own integration tests fairly easily:
All they need to do is create a new
test/integration/{PackageName}.jl
file and add it to the list.@gdalle maybe you could rebase #1563 on this so we can have them in the same format?