Skip to content
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

Reset threads at the beginning of every simulation #1583

Closed
efaulhaber opened this issue Jul 23, 2023 · 5 comments · Fixed by #1584
Closed

Reset threads at the beginning of every simulation #1583

efaulhaber opened this issue Jul 23, 2023 · 5 comments · Fixed by #1584

Comments

@efaulhaber
Copy link
Member

For people interested in this issue: a function Polyester.reset_threads!() has just been released to make it easier for people to do this manually (without requiring to depend on PolyesterWeave.jl and ThreadingUtilities.jl explicitly).

Originally posted by @ranocha in JuliaSIMD/Polyester.jl#30 (comment)

With this, I suggest to always call reset_threads! at the beginning of each simulation from within Trixi.

When Trixi crashes due to an error inside a threaded loop, and sometimes when one interrupts a simulation, threading breaks. I got used to calling the PolyesterWeave.jl and ThreadingUtilities.jl version manually every time I interrupt a simulation to avoid running non-threaded simulations accidentally. I thought about doing that automatically in TrixiParticles, but adding more dependencies was a big downside. Now, I feel like it's a no-brainer.

@ranocha
Copy link
Member

ranocha commented Jul 23, 2023

Sounds reasonable - where would you like to do that? I am also not sure whether there should be an option to disable it.

@efaulhaber
Copy link
Member Author

Good question. It would be nice if that happened right in the solve, but then it would have to be in the summary callback, so you'd have to use that.

Instead, maybe put it in the semidiscretize? I guess it's really unlikely that someone breaks the threading and then only calls solve again.

@ranocha
Copy link
Member

ranocha commented Jul 24, 2023

When I'm breaking threading on purpose (e.g., by running different simulation setups and recording the time when they crash), I usually don't use a SummaryCallback to avoid cluttering the console. Thus, semidiscretize appears to be a reasonable place. We need to remember that there are several methods of semidiscretize we need to take care of:

  semidiscretize(semi::AbstractSemidiscretization, tspan)

  Wrap the semidiscretization semi as an ODE problem in the time interval tspan that can be passed to
  solve from the SciML ecosystem (https://diffeq.sciml.ai/latest/).

  ────────────────────────────────────────────────────────────────────────────────────────────────────────

  semidiscretize(semi::AbstractSemidiscretization, tspan, restart_file::AbstractString)

  Wrap the semidiscretization semi as an ODE problem in the time interval tspan that can be passed to
  solve from the SciML ecosystem (https://diffeq.sciml.ai/latest/). The initial condition etc. is taken
  from the restart_file.

  ────────────────────────────────────────────────────────────────────────────────────────────────────────

  semidiscretize(semi::SemidiscretizationHyperbolicParabolic, tspan)

  Wrap the semidiscretization semi as a split ODE problem in the time interval tspan that can be passed to
  solve from the SciML ecosystem (https://diffeq.sciml.ai/latest/). The parabolic right-hand side is the
  first function of the split ODE problem and will be used by default by the implicit part of IMEX methods
  from the SciML ecosystem.

On the other hand, it also breaks the single-purpose rule since semidiscretize then does something that has nothing to do with its documented behavior...
What do other @trixi-framework/developers think about this?

@ranocha
Copy link
Member

ranocha commented Jul 24, 2023

For the record: The required version of Polyester.jl for this is v0.7.5.

@sloede
Copy link
Member

sloede commented Jul 24, 2023

it also breaks the single-purpose rule

I agree. However, as you pointed out, there is no perfect (or "clean") place for this unless we want to add another callback or something similar, which is akin to just adding the reset statement directly to each elixir.

semidiscretize seems to be one of the places that need to be aware of GPU offloading, at least when looking at #1485. Thus making it aware of multithreading as well might not to be too much of a stretch.

To keep it flexible, we could consider adding it as a kwarg to semidiscretize, either as init_multithreading::Bool=true or, to be more flexible, as init_multithreading::Function=reset_multithreading. The latter version would allow to also pass other functions to "initialize multithreading", but might also be more than what is currently needed (reset_multithreading would then be of course a function that does the proposed resetting and would be implemented in Trixi proper).

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 a pull request may close this issue.

3 participants