-
Notifications
You must be signed in to change notification settings - Fork 779
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
Improvements in linear #1091
Improvements in linear #1091
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A bunch of comments but LGTM.
* VectorValues given = ...; | ||
* auto sample = gbn.sample(given, &rng); | ||
*/ | ||
VectorValues sample(VectorValues given, std::mt19937_64* rng) const; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I have a better suggestion. You can change this signature to
VectorValues sample(VectorValues given, std::mt19937_64 rng = std::mt19937_64(42)) const;
and the definition would have us pass &rng
. I tried it out locally and this works great!
Con: You will have convert from a pointer to a reference pass-by-value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sticking to my solution, as it uses one static rng in cpp.
Don't spend too much time on this.
Improvements in linear: