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

FEATURE: fixed dt #2053

Merged
merged 166 commits into from
Apr 5, 2023
Merged

FEATURE: fixed dt #2053

merged 166 commits into from
Apr 5, 2023

Conversation

boeschf
Copy link
Contributor

@boeschf boeschf commented Dec 1, 2022

Motivation

So far, multi-compartment cable cells use a variable time step: the user-specified maximum time step $dt$ may be shortened based on incoming events, such as spikes, generators, and (exact) sampling, so that the event delivery is performed exactly at the requested times.

Proposal: All events with time $t_s$ are now delivered at simulation time $t_i = t_0 + i ~dt$ if
$t_s \in \left[ t_i , t_i + dt\right).$

Removing exact delivery increases the speed of the simulation due to elimination of small steps, makes the numerics independent of presence of sampling, and also leads to a number of code simplifications. In particular, we have no more need for

  • integration domains,
  • event binning,
  • sampling policies.

Main Changes

The new event delivery logic is mainly implemented in mc_cell_group::advance and fvm_lowered_cell_impl<Backend>::integrate.

Incidental Changes

  • arb_mechanism_ppack now exposes a scalar time and scalar time step: t and dt
    • All cells advance with the same increments, i.e. no separate integration domains
  • Deliverable events now organized in vector of vector of events. Outermost vector has one element per mechanism_id, inner vector has one element per time step interval.
    • This vector is then used to initialize a event_stream for each shared_state::mech_storage, while earlier, we had only one multi_event_stream per shared_state.
    • Helps with simpler and faster event delivery, and exposes more parallelism on the GPU
  • GPU event_stream is now nearly identical to multi-core implementation (marking etc. is done on CPU) except for optional stable sorting according to mechanism_index, which is done in parallel using a thread_group.
    • Less maintenance
  • shared_state gains some responsibility to handle events: register_events, mark_events, deliver_events
    • Better encapsulation

TODO

  • More documentation (markdown docs, code docs, and rst)
  • Investigate use of grouping within multi_event stream<sampling_event>
  • Performance measurements

Performance Measurements

result_speedup_gpu

Performance with respect to master branch measured on Piz Daint's GPU partition for the busyring benchmark with the following parameters:

  • number of cells: 2000,4000,8000,16000,32000
  • ring size: 10,100,1000
  • duration: 30
  • dt: 0.025
  • min_delay: 2
  • number of synapses: 2000,4000

Each benchmark run was repeated 20 times with a different node allocation. Error bar show 95% confidence interval.

@boeschf boeschf requested a review from thorstenhater March 7, 2023 08:29
Copy link
Contributor

@thorstenhater thorstenhater left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few nits have been picked.

arbor/backends/gpu/matrix_fine.cu Outdated Show resolved Hide resolved
arbor/backends/gpu/stimulus.cu Outdated Show resolved Hide resolved
arbor/include/arbor/gpu/gpu_common.hpp Outdated Show resolved Hide resolved
@boeschf boeschf requested a review from thorstenhater March 13, 2023 09:55
thorstenhater
thorstenhater previously approved these changes Mar 13, 2023
Copy link
Contributor

@thorstenhater thorstenhater left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, it's a nice change, simpler and faster.

thorstenhater
thorstenhater previously approved these changes Mar 24, 2023
Copy link
Contributor

@thorstenhater thorstenhater left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Get this merged!

example/busyring/ring.cpp Outdated Show resolved Hide resolved
thorstenhater
thorstenhater previously approved these changes Mar 28, 2023
Copy link
Contributor

@thorstenhater thorstenhater left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now merge. We'll do the convergence after.

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 this pull request may close these issues.

3 participants