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

Get rid of StableRNG? #6

Closed
PallHaraldsson opened this issue Jan 6, 2022 · 2 comments
Closed

Get rid of StableRNG? #6

PallHaraldsson opened this issue Jan 6, 2022 · 2 comments
Labels
enhancement New feature or request

Comments

@PallHaraldsson
Copy link

PallHaraldsson commented Jan 6, 2022

Hi, I noticed you depend on StableRNG in both your packages, and use for tests (very valid, was made for that, and I thought not much else), but also in src/ for simulation. It may not be a big deal if it's not speed-critical, but I think using the default rng in non-test is going to be much faster (and higher quality). And then you can lose one (admittedly cheap, for startup-cost) dependency. [FYI: You can keep StableRNG for tests only, and for that purpose have a separate Project.toml file.]

FYI: The default rng changed in Julia 1.7, and it's much faster and multi-threaded. There's also other option, that may or may not be needed. If you have any questions please ask me. If this was intentional, just close the issue (your call if you explain).

@fipelle
Copy link
Owner

fipelle commented Jan 7, 2022

Hi Páll,

Thanks for taking the time to take a look at my time series packages.

StableRNG is quite handy for testing and I am using it almost entirely for this very purpose. I have employed it in a few extra functions because I needed to have consistent results while porting the code from the old Julia LTS version to the current one. These implementations are not slow or performance critical, but an extra kick is always welcome. Thus, I will definitely evaluate the default rng going forward!

Thanks for your feedback.

@fipelle fipelle added the enhancement New feature or request label Jan 7, 2022
@fipelle
Copy link
Owner

fipelle commented Jan 7, 2022

I am moving it to a private todo list and closing the issue. I should get back to it soon.

@fipelle fipelle closed this as completed Jan 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants