-
Notifications
You must be signed in to change notification settings - Fork 28
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
Refactor Random Strategy II #352
Conversation
@dlinzner-bcs @Osburg |
I investigated this a bit more, but I have no idea what is going on here :D, the sampling procedure is from what I see exactly the same but in the original case, we have 7 zero eigenvalues and here we have 8. No idea why. |
Hi @jduerholt,
I think we should not do this since it creates regular patterns in the drawn values between the features.
I guess similar (possibly undetected) issues may occur in other places where sampling methods are called multiple times with the same seed. |
@Osburg, thank you very much for the catch! I overlooked it when refactoring the random strategy! |
@bertiqwerty: it is fixed now and ready for review! |
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.
Thanks, Johannes.
In this PR the
PolytopeSampler
Strategy is removed and completely integrated into the RandomStrategy, furthermoreUniversalConstraintSampler
is renamed intoSpaceFillingStrategy
as discussed on Friday.