-
Notifications
You must be signed in to change notification settings - Fork 15
Julia #180
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
Conversation
Switch to Bumper for Managing Temporaries
* f32 failure test * tests for grad and Q * cleanup tests
JL: Improve GPU Interface
@Luthaf -- sorry I missed your message. Thank you, No proper review needed, just a sanity check that I'm not messing with anything in main. |
The code looks fine, but CI seems broken. I'll investiguate what's going on! Is this all the changes you want to make & merge for now? |
The Is that ok for you? |
Yes, this is fine. The CI error seems relevant to these changes:
|
that's weird, I thought I had already fixed this. I'll figure this out and ping you again when it is ready to merge. |
@Luthaf -- unsure why this still fails, I only changed the Julia version in the CI script. But the failure seems to be elsehwere now? |
Yes, the new failure is unrelated, this is a new version of clang-format not being happy with our C++ code. I'll fix it separately and update this branch! |
Tests now fail on macOS with
Should I remove the macOS tests or is the OS still supported? |
there are multiple problems that I don't see locally. I prefer to fix them before merging. |
I suspect the Julia test action is not correctly specified. I normally just use defaults provided by the Juli apackage manager, I'll have to look into it. |
The problem is that this line
doesn't actually run the Julia tests correctly as it doesn't load the test dependencies. I tried to modify this as I think is correct. If this fails, then we may need to setup a separate test action and we can copy-paste in the standard Julia test script. |
tests seem to run ok now now I get a function call error that I need to track down. Weird how I don't see any of this locally. |
@Luthaf -- fingers crossed I fixed the final problem. It also seems there might be a time-out problem on one of the tests? Not sure how to fix the test script. And have I re-introduced old problems with the other CI scripts? |
Everything looks good!
What makes you think so? If it is the red cross on the merge commit, this is because you pushed another commit before CI could finish, so it canceled the first CI run. But then CI for the latest commit ran to completion! |
I thought I saw some other errors, but great that everything passed now. From my end this is then ready to merge. Thank you for your patience. |
Just a regular PR to keep
main
andjulia
synchronized.