-
-
Notifications
You must be signed in to change notification settings - Fork 455
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
Consider adding an owned version of WeightedChoice #90
Comments
I would really like to see this, too. |
I think it might even be better to provide only an owned version. The borrowed version does not seem to provide many useful advantages over an owned version. For example, you can't re-use the On the other hand, the borrowed version allows building a |
I changed to this, which passes the test suite: pub struct WeightedChoice<T: AsMut<[Weighted<U>]> + AsRef<[Weighted<U>]>, U: Clone> {
items: T,
weight_range: Range<u32>,
_phantom: marker::PhantomData<U>,
} But it looks a bit crazy. |
Opened a PR #151 to get the ball rolling. |
Haha quick work! Yes yours looks more proper, and non-breaking! |
Hm build failed for mine, but |
Not an argument for my specific implementation, but one could argue that the current version of WeightedChoice is not very idiomatic and perhaps should be retired. The design seems awkward. |
The change to WeightedChoice is debatable, but since the argument must normally be constructed specially anyway, the reference model makes little sense. See issues rust-random#90, rust-random#142 and PR rust-random#152.
Notable changes: * transform::rand::Seed got simpler * Misc minor API differences * Swap out rand::distributions::WeightedChoice with custom WeightedIndexes type WeightedChoice for released version of rand does not own the array it operates on. Because of this, it is not possible to keep around an instance of WeightedChoice and reuse its internal range. The experimental version of rand resolves this but it cannot be used by published crates. Fix by rolling our own WeightedIndexes type. See rand issues: * rust-random/rand#90 * rust-random/rand#142
This API was deprecated and replaced with a new API in #518 which I think fixes this. |
This can more easily embedded within any other type, than the WeightedChoice type.
https://github.com/mneumann/graph-annealing/blob/master/src/owned_weighted_choice.rs
Or, maybe better would be to add code to reuse some code of weighted_choice?
The text was updated successfully, but these errors were encountered: