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

Add support for randomness customization #13

Merged
merged 2 commits into from
Jan 7, 2024
Merged

Add support for randomness customization #13

merged 2 commits into from
Jan 7, 2024

Conversation

Caellian
Copy link
Contributor

@Caellian Caellian commented Jul 27, 2023

Instead of forcing Pcg64Mcg. This PR adds ability to pass in a custom random generator for each model component (Generator, Excl. Gateway, Stochastic Gate, Processor), if None is provided a global rng will be used as previously.

I'm using Rc<RefCell<dyn Debug + RngCore>> for storage which is required because of possible type variance of rng's in Model structs (generics wouldn't work; requires variadic generics). As a side effect, this allows multiple Models to share the same rng.

I tested a simulation using this change and it didn't break. Not sure about performance impact, but simulation performance should be a bit worse because of the added jump and vtable lookup each time rng was previously used.
Let me know whether there's anything you'd like to change/improve in what I've done in this PR so far, or it's okay as it is.

Before merging I'd like to rename uniform_rng.rs to something else, or possibly move its contents to parent mod.rs. I'd appreciate your opinion on this.

Maybe unwanted changes

  • Services struct now skips deserializing the randomness source as well (and constructs the default).
    • Given that any RngCore struct can be the global rng doesn't make sense to deserialize it anyway. Manually calling the (currently non-existant) rng setter seems like a better option.

Signed-off-by: Tin Svagelj <tin.svagelj@live.com>
Add dyn_rng method for SimulationRng wrapping

Signed-off-by: Tin Svagelj <tin.svagelj@live.com>
@Caellian Caellian changed the title [WIP] Add support for randomness customization Add support for randomness customization Jul 27, 2023
@ndebuhr ndebuhr merged commit 0830470 into ndebuhr:main Jan 7, 2024
@ndebuhr
Copy link
Owner

ndebuhr commented Jan 7, 2024

This is great, thanks @Caellian. Sorry for the delayed response. Merged, with test/clippy regressions fixed and uniform_rng renamed to dynamic_rng.

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.

2 participants