-
Notifications
You must be signed in to change notification settings - Fork 6
Add function reinit_task
#58
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
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.
Thanks! This looks good to me.
rerunning ci |
Looks like everything is passing except nightly macOS. |
The segfault is somewhat concerning (as is the use of deprecated LLVM code). We really should replace the hand-rolled LLVM with Julia equivalents since LLVM has absolutely no stability. |
I had several segfaults when I ran the tests locally, all from the "ThreadingUtilities" testset. Which deprecated LLVM code are you talking about? |
If you look at the CI logs, there's a bunch of
but really the whole https://github.com/JuliaSIMD/ThreadingUtilities.jl/blob/main/src/atomics.jl should be written in Julia rather than LLVM. |
I wonder if the problem is
|
This stuff is unfortunately way out of the scope of my expertise. |
Since this PR doesn't change the code with deprecated LLVM features, I would prefer merging it, as it represents a strict improvement and is required to fix some issues with Polyester.jl. |
JuliaSIMD/Polyester.jl#154 caused Trixi.jl CI to freeze sometimes.
I don't have a MWE for this, but the solution is to only re-initialize failed tasks.
To summarize the previous developments:
ThreadingUtilities.checktask
did not throw an error, which means that threads with errors just failed silently (Threads fail silently on error Polyester.jl#153).checktask
when a task failed #54 changed this behavior to throw an error.Polyester.reset_threads!
also usedchecktask
, which now throws errors, so #154 changed this to callThreadingUtilities.initialize_task
instead ofchecktask
. However, this initializes all tasks, not just the failed ones like before all of these PRs. Apparently, this can cause freezing.reinit_task
, which is the same as the oldchecktask
and Reset only failed tasks inreset_threads!
Polyester.jl#160 uses this inreset_threads!
.