-
Notifications
You must be signed in to change notification settings - Fork 140
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
Remove randomness by default of BBox strategies #1069
base: master
Are you sure you want to change the base?
Conversation
…)" This reverts commit 936da04.
public DomOverWDegRef(V[] variables, long seed) { | ||
super(variables, seed); | ||
public DomOverWDegRef(V[] variables) { | ||
this(variables, (v1, v2) -> 0, 32); |
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.
J'aurais mis la lambda dans une constante pour que ce soit plus lisible / réutilisé
public final static Comparator<V> NO_TIE_BREAK = (v1, v2) -> 0;
Ou plutôt LEX_TIE_BREAK
si par conséquence on a un résultat ordonné en lexico (j'ai eu un doute).
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.
Je vois que le 32 est utilisé à différents endroits, je l'aurais aussi mis en variable static (pas final) DEFAULT_FLUSH_RATE
Pour le suivi des modifications, si demain on passe à 50, c'est plus simple d'associer cela à un nom...
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.
Harmoniser avec SearchParams
public DomOverWDeg(V[] variables, long seed) { | ||
this(variables, seed, Integer.MAX_VALUE); | ||
public DomOverWDeg(V[] variables) { | ||
this(variables, (v1, v2)->0, 32); |
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.
idem
switch (tiebreaker) { | ||
default: | ||
case LEX: | ||
tie = (v1, v2) -> 0; |
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.
here too
Consider this as a fresh start of #1066.
I will add some graphs when I found enough time to run benchmarks.
So, the idea here is as much about the feature itself and about not losing it on my computer 😄
Reverts #1068