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

Use CBRNG for Schedules #2243

Open
thorstenhater opened this issue Jan 2, 2024 · 0 comments · May be fixed by #2386
Open

Use CBRNG for Schedules #2243

thorstenhater opened this issue Jan 2, 2024 · 0 comments · May be fixed by #2386

Comments

@thorstenhater
Copy link
Contributor

thorstenhater commented Jan 2, 2024

Use the CBRNG instead of std::random for schedules and other random draws.
Mainly, that'll affect the Poisson schedule.

As we have taken R123 on as a dependency anyhow, we can leverage it in all relevant
places. By doing this, we reduce the memory footprint of the Poisson schedule's state
and simplify it to a single number. It might be slightly faster, too.

Also, add the PRNG state to SERDES, something we cannot do today.

@thorstenhater thorstenhater mentioned this issue Jan 2, 2024
13 tasks
@thorstenhater thorstenhater removed the bug label Jan 4, 2024
thorstenhater added a commit that referenced this issue Jan 19, 2024
The core issue here is to add units to the user facing API. I decided on
using the LLNL/units
library, which offers conversion and checking at runtime. Runtime is a
requirement -- as much
as I love static guarantees --, but keeping the interface uniform
between Python and C++ is a
must.

While setting this up, I noticed the severe lack of IDE/LSP support for
Arbor, so I added typing
stubs using https:/sizmailov/pybind11-stubgen. The
conjunction of typing and units
exposed misuse of pybind11 in several places, so next I had to massage
the ordering of bindings,
adjust the specification of default arguments, and add the odd missing
binding.

The schedule/event generator interface was tightened up, hiding the
`*_impl` structs and exposing
only the type erased `schedule` object. That in turn required
de-generification of the Poisson
schedule. Now, Mersenne twister is the only choice and I will remove
that later on for the CBRNG
we are already using elsewhere.

Currently, units are used for:
- [X] simulation
- [X] schedule/generator
- [x] paintables
- [X] placeables
  - [X] iclamp
  - [X] threshold
- [X] connections
- [X] gap junctions

Adding units to mechanism interfaces is _interesting_ but requires more
work and thought, so
I'll defer that to a later point in time. We'd need to adjust modcc to
expose and **check** units
and devise a scheme to handle missing units.

Generic TODOs; some might spin off into separate issues.
- [x] ~~rename py::iclamp OR cpp::i_clamp for consistency~~ covered by
#2239
- [x] use scale/base for iexpr paintables for consistency with
scaled_mech
- [x] ~~Use CBRNG for Poisson schedule~~ covered by #2243 
- [ ] Automate stub generation. A wishlist item, requires installing
extra software.
- [x] Properly integrate units w/ spack. NB. Units doesn't have a
spackage.

Closes #1983 
Closes #2032

---------

Co-authored-by: boeschf <[email protected]>
@thorstenhater thorstenhater linked a pull request Aug 15, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant