search

ibex-team / ibex-lib

IBEX is a C++ library for constraint processing over real numbers.
http://ibex-team.github.io/ibex-lib/
GNU Lesser General Public License v3.0
67 stars 51 forks source link

Clean CellOptimHeap and OptimCell #46

Closed Jordan08 closed 9 years ago

Jordan08 commented 10 years ago

Due to the management of two heaps Many think has been introduce. I will clean it

gchabert commented 9 years ago

Petite question: pourquoi CellHeapElt contient un indice (qui est, si j'ai bien compris, son numéro dans chaque tas) et pas directement un lien vers le CellHeapNode qui le contient? Cela éviterait de devoir faire ensuite un getnode(i) qui établit un parcours (certes, en temps log).

gchabert commented 9 years ago

J'ai renommé CellHeap_2 CellSharedHeap, je me suis dit que ça te plaraît. Je propose que le champ criterion de cette classe soit redescendu au niveau de la sous-classe CellHeapCost.

gchabert commented 9 years ago

Idem, que goal_var apparaisse dans les sous-classes. Il y a une légère duplication mais c'est plus clean d'un point de vue objet.

gchabert commented 9 years ago

Encore une petite question: pourquoi CellDoubleHeap::erase_other_heaps appelle dans certains cas erase_node et dans d'autre erase_node_no_update?

Jordan08 commented 9 years ago

OK

Par contre, je préfère garder les indice. Ça peut apporter de l'info et je trouve ça plus concret. et si on change ça faut changer beaucoup de trucs un peu partout... A voir.... Le 19 déc. 2014 12:54, "Gilles Chabert" notifications@github.com a écrit :

Idem, que goal_var apparaisse dans les sous-classes. Il y a une légère duplication mais c'est plus clean d'un point de vue objet.

— Reply to this email directly or view it on GitHub https://github.com/ibex-team/ibex-lib/issues/46#issuecomment-67629766.

Jordan08 commented 9 years ago

Oui c'est important car dans certain cas, Lorsque le loup change, il faut tout trier et dans d'autre cas une simple mise a jour et suffisante. La, c'est normale. Faut pas modifier ce truc

A+ Le 19 déc. 2014 13:04, "Gilles Chabert" notifications@github.com a écrit :

Encore une petite question: pourquoi CellDoubleHeap::erase_other_heaps appelle dans certains cas erase_node et dans d'autre erase_node_no_update?

— Reply to this email directly or view it on GitHub https://github.com/ibex-team/ibex-lib/issues/46#issuecomment-67630554.

gchabert commented 9 years ago

OK, certains critères sont sensibles à un changement de loup, d'autres non.. je mettrai ça en commentaire dans le code.

Sur l'organisation générale du code, j'ai une proposition à te faire: séparer la partie propre à l'optimisation de la partie structure de données pure. L'idée est la suivante: faire une structure SharedHeap générique dans tools/ basée sur des HeapNode et HeapElement, cette classe contenant les champs indices/crit génériques et la donnée spécifique T* (qui sera utilisée avec T=Cell). Je m'occuperai de tout, j'ai d'ailleurs déjà fait cette manip pour la structure CellHeap de base, voir la branche heap-tools: https://github.com/ibex-team/ibex-lib/tree/heap-tools.

Mais il y a un point plus prioritaire: les tests de nonreg ne passent pas du tout actuellement sur hotfix-46, le premier bench mouline pendant 10 minutes: https://travis-ci.org/ibex-team/ibex-lib/builds/44802090. Peux-tu regarder ça?

Gilles

Jordan08 commented 9 years ago

c'est une très bonne idée pour la structure generique. Je vote pour ;-)

Pour la branche hotfix-46, je regarderai ça la semaine prochaine, car là je n'ai plus d'ordi linux sous la main.

En tout cas, c'est une super nouvelle pour les tests de nonreg. C'est cool que tout passe correctement.

Bonne fête de fin d'année!!!!

gchabert commented 9 years ago

ok super. Bonnes fêtes à toi aussi!

Jordan08 commented 9 years ago

J4ai pas mal modifié la structure.

J'ai nottamment fusionné entierrement Heap<T> et le CellHeap que j'avais fait.

Et ça marche :+1:

Mais j'ai besoin d'un regard exterieur sur l'architecture Merci d'avance

gchabert commented 9 years ago

ok, je regarde!

gchabert commented 9 years ago

Salut Jordan Je vois que tu as finalement viré l'implémentation d'un tas à partir de la STL (sort_heap). A la base je pensais qu'il y aurait deux structures de données: Heap (tas simple, basée sur la STL) et SharedHeap (structure de tas partagée, fait maison). Pourquoi as-tu préféré fusionner finalement? Je ne suis pas forcément contre, je veux juste comprendre. Si on maintient ça, je propose de renommer ibex_SharedHeap.cpp en ibex_OptimHeap.cpp du coup (vu que tout tas est désormais partageable; la particularité de ces tas-là sont qu'ils sont liés à l'optim).

L'une des raisons qui motivent ce commentaire est que les tests passent déjà beaucoup mieux (en effet) mais qu'il reste une différence non négligeable sur le nombre de cellules, signe que les tas ne fonctionnent pas tout à fait comme avant: https://www.facebook.com/video.php?v=863393173711675 Cela dit, les temps sont tous OK (les temps sont testés avant le nombre de boîtes).

A+

Jordan08 commented 9 years ago

Salut Gilles,

Ce travail est né d'une frustation: je ne comprenais pas pourquoi ça ne marchait pas mieux.... Du coup, j'ai tou repris et fusionné le tout, je trouve ça beaucoup plus cohérent. Et ça marche! :D (la vidéo est trop bien) Pour le nombre de boîte, c'est normale que l'on ne fait pas exactement le même nombre, car il y a toujours des boite avec la borne inf au début. Du coup, l'ordre doit changer un peu. Mais au moins, même en parcourant plus de boîte on est dans les temps.

Pour le changement de nom, je vote pour. De tout façon le fichier ibex_SharedHeap.cpp est une collection de Heap. Donc on peut mettre le nom que l'on veut.

D'une point de vu génie logiciel, j'ai peut-être mis trop de méthode en "virtual" dans ibex_Heap J'ai nottamment eu des problème pour faire le "operator<<", je suis passé par une méthode "print". Qu'en penses-tu?

a+ ps: si tu est en train de regarder ce que j'ai fait, je te conseille les branches "standalone", "hotfix-106" et "hotfix-9". C'est celle dont les modifs ont le moins d'impact.

gchabert commented 9 years ago

Salut Jordan, Désolé pour la vidéo, un mauvais copier-coller :-D Je préfère continuer sur hotfix-46, vu que j'ai commencé. Je vais faire pas mal de modif, n'y touche plus... A+

Jordan08 commented 9 years ago

Pourtant je trouvais la vidéo de circonstance ;-)

ok je touche plus à la hotfix-46. D'un autre côté, vu que déjà à moi je n'étais pas du tout satisfait de certain truc, je me doutais bien que pour toi, il devait y avoir quelque hérésie dans mon code ;-)

a+

gchabert commented 9 years ago

Pas d'hérésie... le code est correct et testé, c'est déjà super :) Je retravaille pas mal par contre la structuration. C'est laborieux mais j'avance... Je te présenterai les modifs majeures bientôt. En attendant, juste deux petites questions sur ibex_DoubleHeap.cpp:

A+ Gilles

Jordan08 commented 9 years ago

Salut,

En fait, c'est un truc pour éviter d'avoir une méthode "setLoup" qui n'a pas de sens pour certaine DoubleHeap. Du coup, la seul façon de modifier le "loup" c'est d'utiliser "contractHeap" C'est pour ça que je l'utilise ici sur une Heap vide. Qu'en penses-tu? Ca peut peut-etre creer des problmeme dans le fututr, non?

gchabert commented 9 years ago

Salut Jordan, Bon ça y est, j'ai pushé ma proposition. Travis a planté pour une raison bizarre (https://travis-ci.org/ibex-team/ibex-lib/builds/53019291) mais tout à l'air d'aller chez moi. En gros, les points majeurs sont les suivants:

Je crois avoir fait le tour; le reste est d'ordre cosmétique...

Je te laisse regarder... si ça te va on peut basculer tout ça sur develop.

A+ Gilles

Jordan08 commented 9 years ago

Salut,

A première vue, j'aime vraiment mieux ta structure. J'étais pas satisfait de mes "init_copy" et je trouvais qu'il y avait des choses bancales dans l'architecture. C'est une super idée d'avoir introduit les CostFunc, ça simplifie grandement. Heureusement que l'on a un vrai informaticien dans l'équipe ;-)

Je regarde le tout en détail demain, mais ça a l'air vraiment mieux. Par contre, je ne comprends pas non plus le problème qu'il y a sur Travis?? C'est peut-être un crash chez eux?

a+ Jordan

gchabert commented 9 years ago

Merci Jordan... C'est cool d'avoir des collaborateurs souples et ouvert d'esprit :) J'ai eu un doute sur le fait de remplacer les "contract" par "set_loup" dans CellDoubleHeap::contract(...). Dis-moi s'il y a un souci. De même, j'ai mis en dur pour le moment dans le constructeur de DoubleHeap les champs "updateCost" des deux tas à false, je suis en train de régler ce point. A+ Gilles

Jordan08 commented 9 years ago

ok je ferai attention à ces 2 points. Je reprendrais la main dessus seulement vendredi après-midi. D'ici là, modifie ce que tu veux. a+ Jordan

gchabert commented 9 years ago

OK. Du coup, j'ai sorti la question de la mise à jour des coûts en dehors de SharedHeap/DoubleHeap, ce qui a pour résultat que les fonctions "sort" et "contract" prenne en argument l'info (c.a.d., est-ce qu'il faut mettre à jour des coûts ou non?). Et du coup, ça simplifie grandement le contract de CellDoubleHeap. Mais bon, il vaut mieux malgré tout que tu y jettes un coup d'oeil :-) Je pense que je m'en tiens là pour le moment... Bises! Gilles

Jordan08 commented 9 years ago

Salut Gilles, Tout ça me parait fort bien! J'ai relu tout le code et j'ai que 3 remarques:

Qu'en penses-tu? a+

gchabert commented 9 years ago

Salut Jordan,

Pourquoi dans SharedHeap ligne 90, "protected" est mis en commentaire? Pour moi, il faudrait que les méthodes plus bas soit bien en protected.*

Je ne sais plus, tu as bien fait de changer :)

Dans CellCostFunc, est ce que l'on pourrait pas mettre la variable depends_on_loup dans CostFunc en la nommant depends_on_global_var (en mettant pas défault à false). Comme ça dans DoubleHeap, l'interface de contract redeviendrai comme avant:void contract(double loup) Je trouve que ça simplifie encore plus. J'ai fait un push de ces modifs.

Tu as raison, c'est déjà beaucoup mieux comme ça! En fait, on peut aller plus loin je pense... en gros, je préférerais remplacer le "depends_on_global_var" par une variable qui indique à une SharedHeap, au moment de sa création, si les objets T* stockés ont un coût qui ne changera jamais (calculés une et une seule fois) où s'ils peuvent avoir un coût qui change "par moments". Le problème, par contre, c'est qu'on restreint cette notion de "par moments" aux appels à contract(...) et sort(...) ce qui est un peu bizarre au fond, mais bon... au moins, la sémantique de cette variable de contrôle serait claire. Qu'en penses-tu?

Il y avait juste un bug dans DoubleHeap::contract l'appel à contract_rec doit se faire avec !update_cost_heap2, car dans contract_rec la valeur du booleen indique si l'on percote ou non (cad si on ne mets pas à jour le cost ou si on ne le met pas à jour).

Exact, bien joué!

Bonne soirée Jordan

Jordan08 commented 9 years ago

Je suis en train de test le tout beaucoup plus en profondeur et j'ai des bug qui resorte Je corrige tout ça et je push

Jordan08 commented 9 years ago

Pour moi, c'est bon pour cette branche. Je pense que l'on peut s'arreter là. C'est déjà très propre.

J'ai rajouté des tests dans TestDoubleHeap et ça a été plutôt simple de les construire le tout.

gchabert commented 9 years ago

On perd quand même plus de 10% sur les temps, pour 8 benchs parmi les 20 :-( https://travis-ci.org/ibex-team/ibex-lib/builds/53981961 Le pire est pour ex8_4_4 je pense où on passe de 204s (tps de référence avec le facteur machine) à 308s.

Jordan08 commented 9 years ago

C'est bizarre, car par rapport au push précédent, j'ai rien changer (sauf le truc sur `depend_on_global`` et les tests)....

Je pense que pour faire des tests de nonreg valable, il faudrait que l'on est un random déterministe. C'est à dire qui donne la même série de nombres aléatoires. Comme ça d'une exécution à l'autre il n'y aurait aucun changement, tout serait déterministe. J'en ai codé un dans src/tools/ibex_Random.h dans la branche "standalone". J'ai un ami qui avait du également codé son random, car il n'arrivait pas à reproduire les bugs qu'on lui signalait (ça dépendait trop de l'aléatoire).

Je pense même que l'on a pas vu le bug du soplex #9 pour cette raison entre autres.

gchabert commented 9 years ago

Le random actuel est déterministe puisqu'on l'initialise avec la même graine à chaque fois (appel à srand(1) dans DefaultOptimizer et OptimizerParam). En fait, les deux variantes du double tas (celle de Bertrand et la tienne) devraient fonctionner exactement de la même manière car il n'y a pas de choix heuristiques à faire dans l'implémentation d'un tas il me semble. Il faudrait donc prendre un test de non-régression qui diffère, l’exécuter en parallèle sur les branches develop et hotfix-46, faire une trace des cellules retirées du tas et remonter à l'origine du problème. C'est fastidieux mais je ne vois pas d'autre solution...

gchabert commented 9 years ago

Cela dit, ça n'explique pas pourquoi les temps passaient (sauf un) dans le commit d'avant... Tu es sûr que les modifs de ton dernier commit ne peuvent pas avoir eu un impact sur les temps de calcul?

Jordan08 commented 9 years ago

entre les deux j'ai simplement:

alors oui ça peut avoir un impact sur les temps, mais il me semblait que dans l'optimizer par default on n'utilisé le tri par LB et UB. Du coup normalement, on utilise jamais le "sort" et donc ça doit être mieux?

a moins que je me sois trompé dans les booleen de "percolate"???

gchabert commented 9 years ago

Hello J'ai fusionné develop pour bénéficier du "home random" comme tu le suggérais. On verra ce que Travis donne cette fois.

Mais je me demande si il faut en attendre grand chose :-( car les calculs de temps d'exécution sont instables. Entre mes deux derniers commits (sans aucun changement impactant l'optimiseur) on obtient sur le même bench un écart passant de +60% à -3%... cf: https://travis-ci.org/ibex-team/ibex-lib/builds/55686169 https://travis-ci.org/ibex-team/ibex-lib/builds/55765620

Petite question en passant: pourquoi y a-t-il un buffer.contract(loup) au début de Optimizer::optimize ?

gchabert commented 9 years ago

Salut Bertrand,

La refonte du double tas dans l'optimiseur touche à sa fin (merci Jordan).

J'aimerais avoir ton feu vert avant de basculer tout dans la branche principale.

Les tests passent bien sous Travis, où tous les temps d'exécution sont en verts (ou presque): https://travis-ci.org/ibex-team/ibex-lib/builds/55768398 Mais je me méfie un peu des temps de calculs, qui semblent assez instables.

Peux-tu faire quelques tests sur ta machine et nous dire si les résultats sont acceptables de ton point de vue.

Pour cela:

git fetch origin git checkout --track origin/hotfix-46

Merci.

A+ Gilles

Jordan08 commented 9 years ago

Pour info voici mes run avec hotfix-46: measuring machine performance... reference time=1.22081 current time=0.70104 ratio=0.574242 ex2_1_9.bch SUCCESS: time=6.37543s [-9.847%] nb_cells=1975 [+3.458%] ex6_1_1.bch SUCCESS: time=18.06s [-1.394%] nb_cells=2613 [+12.2%] ex6_1_3.bch SUCCESS: time=116s [-17.09%] nb_cells=8166 [+11.27%] ex6_2_6.bch SUCCESS: time=30.81s [-27.62%] nb_cells=19900 [+0.6016%] ex6_2_8.bch SUCCESS: time=21.97s [-36.71%] nb_cells=13273 [-0.5991%] ex6_2_9.bch SUCCESS: time=12.59s [-37.01%] nb_cells=6517 [+1.432%] ex6_2_11.bch SUCCESS: time=10.33s [-31.9%] nb_cells=7008 [+2.772%] ex6_2_12.bch SUCCESS: time=8.682s [-41.81%] nb_cells=4864 [+1.249%] ex7_2_3.bch SUCCESS: time=7.774s [-27.83%] nb_cells=2729 [+19.38%] ex7_2_4.bch SUCCESS: time=8.935s [-41.03%] nb_cells=1998 [+4.063%] ex7_2_8.bch SUCCESS: time=7.989s [-43.3%] nb_cells=1911 [-25.95%] ex7_2_9.bch SUCCESS: time=24.62s [-28.58%] nb_cells=4037 [-3.235%] ex7_3_4.bch SUCCESS: time=3.611s [-33.85%] nb_cells=533 [+15.37%] ex7_3_5.bch SUCCESS: time=4.953s [-23.22%] nb_cells=1062 [+62.14%] ex8_4_4.bch SUCCESS: time=84.33s [-13.79%] nb_cells=2647 [+49.72%] ex8_4_5.bch SUCCESS: time=56.78s [-24.78%] nb_cells=1871 [+13.74%] ex8_5_1.bch SUCCESS: time=4.76s [+12.57%] nb_cells=1364 [-0.4379%] ex8_5_2.bch SUCCESS: time=18.2s [+5.604%] nb_cells=4459 [+11.31%] ex8_5_6.bch SUCCESS: time=9.13s [-24.07%] nb_cells=2385 [-7.017%] ex14_2_7.bch SUCCESS: time=18.76s [-14.85%] nb_cells=2201 [-0.6768%] hydro.bch SUCCESS: time=152.3s [-21.78%] nb_cells=6945 [+6.683%] immun.bch FAILED: time=13.59s [+77.37%] nb_cells=2769 [-8.22%] ramsey.bch SUCCESS: time=33.18s [+8.937%] nb_cells=824 [+38.03%] srcpm.bch SUCCESS: time=25.92s [-14.17%] nb_cells=384 [+13.61%]

pour la branch develop: measuring machine performance... reference time=1.22081 current time=0.701269 ratio=0.574429 ex2_1_9.bch SUCCESS: time=5.92158s [-16.29%] nb_cells=1800 [-5.709%] ex6_1_1.bch SUCCESS: time=15.6s [-14.85%] nb_cells=2253 [-3.263%] ex6_1_3.bch SUCCESS: time=90.28s [-35.49%] nb_cells=6567 [-10.51%] ex6_2_6.bch SUCCESS: time=32s [-24.84%] nb_cells=19994 [+1.077%] ex6_2_8.bch SUCCESS: time=22.53s [-35.11%] nb_cells=13303 [-0.3744%] ex6_2_9.bch SUCCESS: time=12.66s [-36.65%] nb_cells=6473 [+0.7471%] ex6_2_11.bch SUCCESS: time=10.48s [-30.94%] nb_cells=7002 [+2.684%] ex6_2_12.bch SUCCESS: time=8.671s [-41.91%] nb_cells=4760 [-0.9159%] ex7_2_3.bch SUCCESS: time=7.767s [-27.91%] nb_cells=2511 [+9.843%] ex7_2_4.bch SUCCESS: time=7.953s [-47.52%] nb_cells=1704 [-11.25%] ex7_2_8.bch SUCCESS: time=11.11s [-21.18%] nb_cells=3087 [+19.61%] ex7_2_9.bch SUCCESS: time=28.64s [-16.95%] nb_cells=4971 [+19.16%] ex7_3_4.bch SUCCESS: time=3.364s [-38.38%] nb_cells=469 [+1.516%] ex7_3_5.bch SUCCESS: time=3.419s [-47.01%] nb_cells=498 [-23.96%] ex8_4_4.bch SUCCESS: time=60.72s [-37.94%] nb_cells=1531 [-13.4%] ex8_4_5.bch SUCCESS: time=57.56s [-23.76%] nb_cells=1789 [+8.754%] ex8_5_1.bch SUCCESS: time=4.697s [+11.05%] nb_cells=1305 [-4.744%] ex8_5_2.bch SUCCESS: time=16.94s [-1.752%] nb_cells=4064 [+1.448%] ex8_5_6.bch SUCCESS: time=8.611s [-28.41%] nb_cells=2748 [+7.135%] ex14_2_7.bch SUCCESS: time=18.41s [-16.45%] nb_cells=2269 [+2.392%] hydro.bch SUCCESS: time=145.3s [-25.4%] nb_cells=6217 [-4.5%] immun.bch FAILED: time=16.1s [+110%] nb_cells=3461 [+14.72%] ramsey.bch SUCCESS: time=37.31s [+22.46%] nb_cells=1009 [+69.02%] srcpm.bch SUCCESS: time=27.25s [-9.794%] nb_cells=392 [+15.98%]

Jordan08 commented 9 years ago

ça varie un peu, mais pour moi, c'est ok. Faut dire que le code est qu'en même plus beau ;-)

bneveu commented 9 years ago

J'ai regardé l'algo d'optim par défaut (le double tas lb ub). Je n'ai pas testé les autres critères (qui ne servent que de comparaison dans l'article en soumission). Les comportements en temps et nombre de boites sont assez similaires, bien qu'un peu différents. Je n'ai pas sur ma machine de gros écarts en temps (pas de gros gains comme semblent l'indiquer tes mesures de performance sous Travis , qui du coup ne semblent plus très fiables. C'est sur la même machine ?

Il me semble que la seule différence dans l'algo est que chaque tas est trié par une seule fonction, alors qu'auparavant, le critère était arbitraire (codé dans le comparateur) et permettait par exemple de trier les ex-aequo d'un tas : c'etait surtout utile quand on avit un seul tas, pour avoir lb en 1er critère et ub en 2ème critère.

Pour les deux tas, c'était peut être effectivement un luxe inutile.

Le code est je pense maintenant plus lisible pour un autre que moi.

A+,

Bertrand.

gchabert commented 9 years ago

Salut Bertrand Merci d'avoir regardé. Effectivement, on a perdu cette possibilité d'utiliser plusieurs critères pour un même tas, ça m'avait échappé... mais je pense que ça vaut le coup de basculer sur le nouveau code malgré tout, sachant qu'on pourra toujours, si le besoin se faire sentir, réimplémenter cette fonctionnalité en donnant à un tas non seulement une fonction de coût (CostFunc) mais également un comparateur.

Je vais fusionner... A+ Gilles PS: On ne sait pas sur quelle machine Travis exécute les tests. Probablement, ce n'est jamais la même.

gchabert commented 9 years ago

J'ai encore fait toute une série de modifications pour améliorer le code (par exemple, j'ai sorti de l'optimiseur la gestion des données "pf" et "pu" spécifiques aux fonctions de coût).

Je pense avoir trouvé un bug au passage: les critères PF_LB et PF_UB utilisent (comme leur nom l'indique) le "pf" mais l'optimiseur ne l'ajoutait pas dans OptimData lorsque ces critères étaient utilisés. Je ne l'ai pas encore corrigé (tout ça se passe maintenant dans CellCostFunc)... vous confirmez?

Au final, je trouve les noms "pf", "pu", "c3" etc. très peu parlants. Ces noms font référence à une référence bibliographique?

Jordan08 commented 9 years ago

Oui en effet, c'est mieux. ça permet vraiment de rajouter des critère avec un minimum de modif. Le code me parrait très modulaire maintenant.

Pour PF_LB et PB_UP, je confirme il faut ajouter les OptimData. Mais si j'ai bien compris, il suffit d'implementer add_backtrackable et set_optim_data dans CellCostFunc.h

Pour les nom, il faut demander à Bertrand. a+

gchabert commented 9 years ago

Exact (c'est fait).

J'ai supprimé la branche hotfix-46, tout est dans develop désormais. A+

bneveu commented 9 years ago

les critères C3, C5, C7 ainsi que les notations pf et pu proviennent de l'article de Markot Fernandez Casado Csendes New interval methods for constrained global optimization dans mathematical programming.

Je les avais implantés pour qu'on puisse se comparer à eux: ils n'ont pas vocation à être fournis et documentés à l'utilisateur d'Ibex.

Par contre, ce serait bien s'ils pouvaient continuer à fonctionner dans les futures versions (on aura peut être à les refaire tourner si notre article est accepté).

Il n'y a pas de critère PF_LB et PF_UB (tu les as ajoutés ?)

pf et pu sont des calculs intermédiaires utilisés par C5 et C7 qui étaient stockés dans la cellule (pf sous forme d'intervalle et pu de simple nombre). J'avais ajouté un critère basé sur pu.

De même, loup, qui correspondait à la valeur du loup au moment du stockage de la cellule dans le tas était aussi stocké dans la cellule

Je ne comprends pas d'où provient le loup dans CellCostC7::cost de la version courante

gchabert commented 9 years ago

Il n'y a pas de critère PF_LB et PF_UB (tu les as ajoutés ?)

Ce n'est pas moi qui les ai ajoutés...

De même, loup, qui correspondait à la valeur du loup au moment du stockage de la cellule dans le tas était aussi stocké dans la cellule

Effectivement, ce n'est plus le cas...le loup est passé à la fonction de coût directement (voir les champs de la classe). Ça me paraissait plus logique car le coup est global et non spécifique à la cellule (contrairement à pf et pu).

Si ça te paraît correct je te laisse clore le "bug".

A+ Gilles

bneveu commented 9 years ago

Le fait d'utliser le loup courant et non le loup au moment de la création de la cellule donne des critères différents de ceux que j'avais implantés, mais c'est sans doute une implantation plus proche des critères C3,C5 et C7 tels qu'ils sont définis.

Ça me paraît correct , à condition que le tas correspondant soit retrié à chaque nouveau loup, ce qui doit être fait par la fonction contract. (le code actuel étant là nettement plus compliqué que l'ancien qui utilisait le heap de la STL , je n'arrive pas à en être convaincu).

Je vais tester ces critères avant de clore le "bug"

A +

Bertrand

gchabert commented 9 years ago

Je me permets de fermer cette issue.